mirror of
https://github.com/openai/codex.git
synced 2026-11-14 21:40:39 +00:00
Merge branch 'etraut/next-turn-state-remove-user-input-with-context' into etraut/next-turn-state-input-op-consolidation
# Conflicts: # codex-rs/core/src/session/handlers.rs # codex-rs/core/tests/common/test_codex.rs # codex-rs/core/tests/suite/request_permissions.rs # codex-rs/core/tests/suite/unified_exec.rs # codex-rs/protocol/src/protocol.rs
This commit is contained in:
@@ -499,13 +499,12 @@ impl AgentControl {
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
});
|
||||
match self
|
||||
.resume_single_agent_from_rollout(
|
||||
config.clone(),
|
||||
child_thread_id,
|
||||
child_session_source,
|
||||
)
|
||||
.await
|
||||
match Box::pin(self.resume_single_agent_from_rollout(
|
||||
config.clone(),
|
||||
child_thread_id,
|
||||
child_session_source,
|
||||
))
|
||||
.await
|
||||
{
|
||||
Ok(_) => true,
|
||||
Err(err) => {
|
||||
|
||||
@@ -487,7 +487,13 @@ async fn send_inter_agent_communication_without_turn_queues_message_without_trig
|
||||
|
||||
timeout(Duration::from_secs(5), async {
|
||||
loop {
|
||||
if thread.codex.session.has_pending_input().await {
|
||||
if thread
|
||||
.codex
|
||||
.session
|
||||
.input_queue
|
||||
.has_pending_input(&thread.codex.session.active_turn)
|
||||
.await
|
||||
{
|
||||
break;
|
||||
}
|
||||
sleep(Duration::from_millis(10)).await;
|
||||
|
||||
@@ -1,161 +0,0 @@
|
||||
use codex_protocol::protocol::InterAgentCommunication;
|
||||
use std::collections::VecDeque;
|
||||
use std::sync::atomic::AtomicU64;
|
||||
use std::sync::atomic::Ordering;
|
||||
use tokio::sync::mpsc;
|
||||
use tokio::sync::watch;
|
||||
|
||||
#[cfg(test)]
|
||||
use codex_protocol::AgentPath;
|
||||
|
||||
pub(crate) struct Mailbox {
|
||||
tx: mpsc::UnboundedSender<InterAgentCommunication>,
|
||||
next_seq: AtomicU64,
|
||||
seq_tx: watch::Sender<u64>,
|
||||
}
|
||||
|
||||
pub(crate) struct MailboxReceiver {
|
||||
rx: mpsc::UnboundedReceiver<InterAgentCommunication>,
|
||||
pending_mails: VecDeque<InterAgentCommunication>,
|
||||
}
|
||||
|
||||
impl Mailbox {
|
||||
pub(crate) fn new() -> (Self, MailboxReceiver) {
|
||||
let (tx, rx) = mpsc::unbounded_channel();
|
||||
let (seq_tx, _) = watch::channel(0);
|
||||
(
|
||||
Self {
|
||||
tx,
|
||||
next_seq: AtomicU64::new(0),
|
||||
seq_tx,
|
||||
},
|
||||
MailboxReceiver {
|
||||
rx,
|
||||
pending_mails: VecDeque::new(),
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
pub(crate) fn subscribe(&self) -> watch::Receiver<u64> {
|
||||
self.seq_tx.subscribe()
|
||||
}
|
||||
|
||||
pub(crate) fn send(&self, communication: InterAgentCommunication) -> u64 {
|
||||
let seq = self.next_seq.fetch_add(1, Ordering::Relaxed) + 1;
|
||||
let _ = self.tx.send(communication);
|
||||
self.seq_tx.send_replace(seq);
|
||||
seq
|
||||
}
|
||||
}
|
||||
|
||||
impl MailboxReceiver {
|
||||
fn sync_pending_mails(&mut self) {
|
||||
while let Ok(mail) = self.rx.try_recv() {
|
||||
self.pending_mails.push_back(mail);
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn has_pending(&mut self) -> bool {
|
||||
self.sync_pending_mails();
|
||||
!self.pending_mails.is_empty()
|
||||
}
|
||||
|
||||
pub(crate) fn has_pending_trigger_turn(&mut self) -> bool {
|
||||
self.sync_pending_mails();
|
||||
self.pending_mails.iter().any(|mail| mail.trigger_turn)
|
||||
}
|
||||
|
||||
pub(crate) fn drain(&mut self) -> Vec<InterAgentCommunication> {
|
||||
self.sync_pending_mails();
|
||||
self.pending_mails.drain(..).collect()
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
fn make_mail(
|
||||
author: AgentPath,
|
||||
recipient: AgentPath,
|
||||
content: &str,
|
||||
trigger_turn: bool,
|
||||
) -> InterAgentCommunication {
|
||||
InterAgentCommunication::new(
|
||||
author,
|
||||
recipient,
|
||||
Vec::new(),
|
||||
content.to_string(),
|
||||
trigger_turn,
|
||||
)
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn mailbox_assigns_monotonic_sequence_numbers() {
|
||||
let (mailbox, _receiver) = Mailbox::new();
|
||||
let mut seq_rx = mailbox.subscribe();
|
||||
|
||||
let seq_a = mailbox.send(make_mail(
|
||||
AgentPath::root(),
|
||||
AgentPath::try_from("/root/worker").expect("agent path"),
|
||||
"one",
|
||||
/*trigger_turn*/ false,
|
||||
));
|
||||
let seq_b = mailbox.send(make_mail(
|
||||
AgentPath::root(),
|
||||
AgentPath::try_from("/root/worker").expect("agent path"),
|
||||
"two",
|
||||
/*trigger_turn*/ false,
|
||||
));
|
||||
|
||||
seq_rx.changed().await.expect("first seq update");
|
||||
assert_eq!(*seq_rx.borrow(), seq_b);
|
||||
assert_eq!(seq_a, 1);
|
||||
assert_eq!(seq_b, 2);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn mailbox_drains_in_delivery_order() {
|
||||
let (mailbox, mut receiver) = Mailbox::new();
|
||||
let mail_one = make_mail(
|
||||
AgentPath::root(),
|
||||
AgentPath::try_from("/root/worker").expect("agent path"),
|
||||
"one",
|
||||
/*trigger_turn*/ false,
|
||||
);
|
||||
let mail_two = make_mail(
|
||||
AgentPath::try_from("/root/worker").expect("agent path"),
|
||||
AgentPath::root(),
|
||||
"two",
|
||||
/*trigger_turn*/ false,
|
||||
);
|
||||
|
||||
mailbox.send(mail_one.clone());
|
||||
mailbox.send(mail_two.clone());
|
||||
|
||||
assert_eq!(receiver.drain(), vec![mail_one, mail_two]);
|
||||
assert!(!receiver.has_pending());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn mailbox_tracks_pending_trigger_turn_mail() {
|
||||
let (mailbox, mut receiver) = Mailbox::new();
|
||||
|
||||
mailbox.send(make_mail(
|
||||
AgentPath::root(),
|
||||
AgentPath::try_from("/root/worker").expect("agent path"),
|
||||
"queued",
|
||||
/*trigger_turn*/ false,
|
||||
));
|
||||
assert!(!receiver.has_pending_trigger_turn());
|
||||
|
||||
mailbox.send(make_mail(
|
||||
AgentPath::root(),
|
||||
AgentPath::try_from("/root/worker").expect("agent path"),
|
||||
"wake",
|
||||
/*trigger_turn*/ true,
|
||||
));
|
||||
assert!(receiver.has_pending_trigger_turn());
|
||||
}
|
||||
}
|
||||
@@ -1,14 +1,11 @@
|
||||
pub(crate) mod agent_resolver;
|
||||
pub(crate) mod control;
|
||||
pub(crate) mod mailbox;
|
||||
mod registry;
|
||||
pub(crate) mod role;
|
||||
pub(crate) mod status;
|
||||
|
||||
pub(crate) use codex_protocol::protocol::AgentStatus;
|
||||
pub(crate) use control::AgentControl;
|
||||
pub(crate) use mailbox::Mailbox;
|
||||
pub(crate) use mailbox::MailboxReceiver;
|
||||
pub(crate) use registry::exceeds_thread_spawn_depth_limit;
|
||||
pub(crate) use registry::next_thread_spawn_depth;
|
||||
pub(crate) use status::agent_status_from_event;
|
||||
|
||||
@@ -23,6 +23,7 @@ use codex_config::merge_toml_values;
|
||||
use codex_config::project_root_markers_from_config;
|
||||
use codex_exec_server::Environment;
|
||||
use codex_exec_server::ExecutorFileSystem;
|
||||
use codex_exec_server::LOCAL_FS;
|
||||
use codex_features::Feature;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use dunce::canonicalize as normalize_path;
|
||||
@@ -58,13 +59,14 @@ impl<'a> AgentsMdManager<'a> {
|
||||
Self { config }
|
||||
}
|
||||
|
||||
pub(crate) fn load_global_instructions(
|
||||
pub(crate) async fn load_global_instructions(
|
||||
fs: &dyn ExecutorFileSystem,
|
||||
codex_dir: Option<&AbsolutePathBuf>,
|
||||
) -> Option<LoadedAgentsMd> {
|
||||
let base = codex_dir?;
|
||||
for candidate in [LOCAL_AGENTS_MD_FILENAME, DEFAULT_AGENTS_MD_FILENAME] {
|
||||
let path = base.join(candidate);
|
||||
if let Ok(contents) = std::fs::read_to_string(&path) {
|
||||
if let Ok(contents) = fs.read_file_text(&path, /*sandbox*/ None).await {
|
||||
let trimmed = contents.trim();
|
||||
if !trimmed.is_empty() {
|
||||
return Some(LoadedAgentsMd {
|
||||
@@ -128,9 +130,11 @@ impl<'a> AgentsMdManager<'a> {
|
||||
|
||||
/// Returns all instruction source files included in the current config.
|
||||
pub async fn instruction_sources(&self, fs: &dyn ExecutorFileSystem) -> Vec<AbsolutePathBuf> {
|
||||
let mut paths = Self::load_global_instructions(Some(&self.config.codex_home))
|
||||
.map(|loaded| vec![loaded.path])
|
||||
.unwrap_or_default();
|
||||
let mut paths =
|
||||
Self::load_global_instructions(LOCAL_FS.as_ref(), Some(&self.config.codex_home))
|
||||
.await
|
||||
.map(|loaded| vec![loaded.path])
|
||||
.unwrap_or_default();
|
||||
match self.agents_md_paths(fs).await {
|
||||
Ok(agents_md_paths) => paths.extend(agents_md_paths),
|
||||
Err(err) => {
|
||||
|
||||
@@ -931,7 +931,7 @@ impl Drop for ModelClientSession {
|
||||
}
|
||||
|
||||
impl ModelClientSession {
|
||||
pub(crate) fn reset_websocket_session(&mut self) {
|
||||
fn reset_websocket_session(&mut self) {
|
||||
self.websocket_session.connection = None;
|
||||
self.websocket_session.last_request = None;
|
||||
self.websocket_session.last_response_rx = None;
|
||||
|
||||
@@ -2,13 +2,10 @@ pub use codex_api::ResponseEvent;
|
||||
use codex_config::types::Personality;
|
||||
use codex_protocol::error::Result;
|
||||
use codex_protocol::models::BaseInstructions;
|
||||
use codex_protocol::models::FunctionCallOutputBody;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_tools::ToolSpec;
|
||||
use futures::Stream;
|
||||
use serde::Deserialize;
|
||||
use serde_json::Value;
|
||||
use std::collections::HashSet;
|
||||
use std::pin::Pin;
|
||||
use std::task::Context;
|
||||
use std::task::Poll;
|
||||
@@ -64,116 +61,10 @@ impl Default for Prompt {
|
||||
|
||||
impl Prompt {
|
||||
pub(crate) fn get_formatted_input(&self) -> Vec<ResponseItem> {
|
||||
let mut input = self.input.clone();
|
||||
|
||||
// when using the *Freeform* apply_patch tool specifically, tool outputs
|
||||
// should be structured text, not json. Do NOT reserialize when using
|
||||
// the Function tool - note that this differs from the check above for
|
||||
// instructions. We declare the result as a named variable for clarity.
|
||||
let is_freeform_apply_patch_tool_present = self.tools.iter().any(|tool| match tool {
|
||||
ToolSpec::Freeform(f) => f.name == "apply_patch",
|
||||
_ => false,
|
||||
});
|
||||
if is_freeform_apply_patch_tool_present {
|
||||
reserialize_shell_outputs(&mut input);
|
||||
}
|
||||
|
||||
input
|
||||
self.input.clone()
|
||||
}
|
||||
}
|
||||
|
||||
fn reserialize_shell_outputs(items: &mut [ResponseItem]) {
|
||||
let mut shell_call_ids: HashSet<String> = HashSet::new();
|
||||
|
||||
items.iter_mut().for_each(|item| match item {
|
||||
ResponseItem::LocalShellCall { call_id, id, .. } => {
|
||||
if let Some(identifier) = call_id.clone().or_else(|| id.clone()) {
|
||||
shell_call_ids.insert(identifier);
|
||||
}
|
||||
}
|
||||
ResponseItem::CustomToolCall {
|
||||
id: _,
|
||||
status: _,
|
||||
call_id,
|
||||
name,
|
||||
input: _,
|
||||
} => {
|
||||
if name == "apply_patch" {
|
||||
shell_call_ids.insert(call_id.clone());
|
||||
}
|
||||
}
|
||||
ResponseItem::FunctionCall { name, call_id, .. }
|
||||
if is_shell_tool_name(name) || name == "apply_patch" =>
|
||||
{
|
||||
shell_call_ids.insert(call_id.clone());
|
||||
}
|
||||
ResponseItem::FunctionCallOutput {
|
||||
call_id, output, ..
|
||||
}
|
||||
| ResponseItem::CustomToolCallOutput {
|
||||
call_id, output, ..
|
||||
} => {
|
||||
if shell_call_ids.remove(call_id)
|
||||
&& let Some(structured) = output
|
||||
.text_content()
|
||||
.and_then(parse_structured_shell_output)
|
||||
{
|
||||
output.body = FunctionCallOutputBody::Text(structured);
|
||||
}
|
||||
}
|
||||
_ => {}
|
||||
})
|
||||
}
|
||||
|
||||
fn is_shell_tool_name(name: &str) -> bool {
|
||||
name == "shell"
|
||||
}
|
||||
|
||||
#[derive(Deserialize)]
|
||||
struct ExecOutputJson {
|
||||
output: String,
|
||||
metadata: ExecOutputMetadataJson,
|
||||
}
|
||||
|
||||
#[derive(Deserialize)]
|
||||
struct ExecOutputMetadataJson {
|
||||
exit_code: i32,
|
||||
duration_seconds: f32,
|
||||
}
|
||||
|
||||
fn parse_structured_shell_output(raw: &str) -> Option<String> {
|
||||
let parsed: ExecOutputJson = serde_json::from_str(raw).ok()?;
|
||||
Some(build_structured_output(&parsed))
|
||||
}
|
||||
|
||||
fn build_structured_output(parsed: &ExecOutputJson) -> String {
|
||||
let mut sections = Vec::new();
|
||||
sections.push(format!("Exit code: {}", parsed.metadata.exit_code));
|
||||
sections.push(format!(
|
||||
"Wall time: {} seconds",
|
||||
parsed.metadata.duration_seconds
|
||||
));
|
||||
|
||||
let mut output = parsed.output.clone();
|
||||
if let Some((stripped, total_lines)) = strip_total_output_header(&parsed.output) {
|
||||
sections.push(format!("Total output lines: {total_lines}"));
|
||||
output = stripped.to_string();
|
||||
}
|
||||
|
||||
sections.push("Output:".to_string());
|
||||
sections.push(output);
|
||||
|
||||
sections.join("\n")
|
||||
}
|
||||
|
||||
fn strip_total_output_header(output: &str) -> Option<(&str, u32)> {
|
||||
let after_prefix = output.strip_prefix("Total output lines: ")?;
|
||||
let (total_segment, remainder) = after_prefix.split_once('\n')?;
|
||||
let total_lines = total_segment.parse::<u32>().ok()?;
|
||||
let remainder = remainder.strip_prefix('\n').unwrap_or(remainder);
|
||||
Some((remainder, total_lines))
|
||||
}
|
||||
|
||||
pub struct ResponseStream {
|
||||
pub(crate) rx_event: mpsc::Receiver<Result<ResponseEvent>>,
|
||||
/// Signals the mapper task that the consumer stopped polling before the
|
||||
|
||||
@@ -3,7 +3,6 @@ use codex_api::ResponsesApiRequest;
|
||||
use codex_api::TextControls;
|
||||
use codex_api::create_text_param_for_request;
|
||||
use codex_protocol::config_types::ServiceTier;
|
||||
use codex_protocol::models::FunctionCallOutputPayload;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
use super::*;
|
||||
@@ -166,65 +165,3 @@ fn serializes_flex_service_tier_when_set() {
|
||||
Some("flex")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn reserializes_shell_outputs_for_function_and_custom_tool_calls() {
|
||||
let raw_output = r#"{"output":"hello","metadata":{"exit_code":0,"duration_seconds":0.5}}"#;
|
||||
let expected_output = "Exit code: 0\nWall time: 0.5 seconds\nOutput:\nhello";
|
||||
let mut items = vec![
|
||||
ResponseItem::FunctionCall {
|
||||
id: None,
|
||||
name: "shell".to_string(),
|
||||
namespace: None,
|
||||
arguments: "{}".to_string(),
|
||||
call_id: "call-1".to_string(),
|
||||
},
|
||||
ResponseItem::FunctionCallOutput {
|
||||
call_id: "call-1".to_string(),
|
||||
output: FunctionCallOutputPayload::from_text(raw_output.to_string()),
|
||||
},
|
||||
ResponseItem::CustomToolCall {
|
||||
id: None,
|
||||
status: None,
|
||||
call_id: "call-2".to_string(),
|
||||
name: "apply_patch".to_string(),
|
||||
input: "*** Begin Patch".to_string(),
|
||||
},
|
||||
ResponseItem::CustomToolCallOutput {
|
||||
call_id: "call-2".to_string(),
|
||||
name: None,
|
||||
output: FunctionCallOutputPayload::from_text(raw_output.to_string()),
|
||||
},
|
||||
];
|
||||
|
||||
reserialize_shell_outputs(&mut items);
|
||||
|
||||
assert_eq!(
|
||||
items,
|
||||
vec![
|
||||
ResponseItem::FunctionCall {
|
||||
id: None,
|
||||
name: "shell".to_string(),
|
||||
namespace: None,
|
||||
arguments: "{}".to_string(),
|
||||
call_id: "call-1".to_string(),
|
||||
},
|
||||
ResponseItem::FunctionCallOutput {
|
||||
call_id: "call-1".to_string(),
|
||||
output: FunctionCallOutputPayload::from_text(expected_output.to_string()),
|
||||
},
|
||||
ResponseItem::CustomToolCall {
|
||||
id: None,
|
||||
status: None,
|
||||
call_id: "call-2".to_string(),
|
||||
name: "apply_patch".to_string(),
|
||||
input: "*** Begin Patch".to_string(),
|
||||
},
|
||||
ResponseItem::CustomToolCallOutput {
|
||||
call_id: "call-2".to_string(),
|
||||
name: None,
|
||||
output: FunctionCallOutputPayload::from_text(expected_output.to_string()),
|
||||
},
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
@@ -144,7 +144,7 @@ impl CodexThread {
|
||||
self.codex.session_loop_termination.clone().await;
|
||||
}
|
||||
|
||||
pub(crate) fn emit_thread_resume_lifecycle(&self) {
|
||||
pub(crate) async fn emit_thread_resume_lifecycle(&self) {
|
||||
for contributor in self
|
||||
.codex
|
||||
.session
|
||||
@@ -152,10 +152,12 @@ impl CodexThread {
|
||||
.extensions
|
||||
.thread_lifecycle_contributors()
|
||||
{
|
||||
contributor.on_thread_resume(codex_extension_api::ThreadResumeInput {
|
||||
session_store: &self.codex.session.services.session_extension_data,
|
||||
thread_store: &self.codex.session.services.thread_extension_data,
|
||||
});
|
||||
contributor
|
||||
.on_thread_resume(codex_extension_api::ThreadResumeInput {
|
||||
session_store: &self.codex.session.services.session_extension_data,
|
||||
thread_store: &self.codex.session.services.thread_extension_data,
|
||||
})
|
||||
.await;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -381,6 +383,7 @@ impl CodexThread {
|
||||
{
|
||||
self.codex
|
||||
.session
|
||||
.input_queue
|
||||
.queue_response_items_for_next_turn(items)
|
||||
.await;
|
||||
self.codex.session.maybe_start_turn_for_pending_work().await;
|
||||
|
||||
@@ -282,7 +282,6 @@ async fn run_compact_task_inner_impl(
|
||||
};
|
||||
sess.replace_compacted_history(new_history, reference_context_item, compacted_item)
|
||||
.await;
|
||||
client_session.reset_websocket_session();
|
||||
sess.recompute_token_usage(&turn_context).await;
|
||||
|
||||
sess.emit_turn_item_completed(&turn_context, compaction_item)
|
||||
|
||||
@@ -3849,6 +3849,63 @@ async fn rebuild_preserving_session_layers_refreshes_plugin_derived_mcp_config()
|
||||
mcp_config.configured_mcp_servers.get("sample"),
|
||||
Some(&http_mcp("https://sample.example/mcp"))
|
||||
);
|
||||
assert_eq!(
|
||||
mcp_config.plugin_ids_by_mcp_server_name,
|
||||
HashMap::from([("sample".to_string(), "sample@test".to_string())])
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn to_mcp_config_omits_plugin_id_when_user_server_shadows_plugin_mcp() -> anyhow::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let plugin_root = codex_home
|
||||
.path()
|
||||
.join("plugins/cache")
|
||||
.join("test/sample/local");
|
||||
std::fs::create_dir_all(plugin_root.join(".codex-plugin"))?;
|
||||
std::fs::write(
|
||||
plugin_root.join(".codex-plugin/plugin.json"),
|
||||
r#"{"name":"sample"}"#,
|
||||
)?;
|
||||
std::fs::write(
|
||||
plugin_root.join(".mcp.json"),
|
||||
r#"{
|
||||
"mcpServers": {
|
||||
"sample": {
|
||||
"type": "http",
|
||||
"url": "https://plugin.example/mcp"
|
||||
}
|
||||
}
|
||||
}"#,
|
||||
)?;
|
||||
std::fs::write(
|
||||
codex_home.path().join(CONFIG_TOML_FILE),
|
||||
r#"
|
||||
[features]
|
||||
plugins = true
|
||||
|
||||
[mcp_servers.sample]
|
||||
url = "https://user.example/mcp"
|
||||
|
||||
[plugins."sample@test"]
|
||||
enabled = true
|
||||
"#,
|
||||
)?;
|
||||
|
||||
let config = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.build()
|
||||
.await?;
|
||||
let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf());
|
||||
let mcp_config = config.to_mcp_config(&plugins_manager).await;
|
||||
|
||||
assert_eq!(
|
||||
mcp_config.configured_mcp_servers.get("sample"),
|
||||
Some(&http_mcp("https://user.example/mcp"))
|
||||
);
|
||||
assert!(mcp_config.plugin_ids_by_mcp_server_name.is_empty());
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -10112,6 +10169,7 @@ usage_hint_enabled = false
|
||||
usage_hint_text = "Custom delegation guidance."
|
||||
root_agent_usage_hint_text = "Root guidance."
|
||||
subagent_usage_hint_text = "Subagent guidance."
|
||||
tool_namespace = "agents"
|
||||
hide_spawn_agent_metadata = true
|
||||
non_code_mode_only = true
|
||||
"#,
|
||||
@@ -10142,6 +10200,10 @@ non_code_mode_only = true
|
||||
config.multi_agent_v2.subagent_usage_hint_text.as_deref(),
|
||||
Some("Subagent guidance.")
|
||||
);
|
||||
assert_eq!(
|
||||
config.multi_agent_v2.tool_namespace.as_deref(),
|
||||
Some("agents")
|
||||
);
|
||||
assert!(config.multi_agent_v2.hide_spawn_agent_metadata);
|
||||
assert!(config.multi_agent_v2.non_code_mode_only);
|
||||
|
||||
@@ -10164,6 +10226,7 @@ usage_hint_enabled = true
|
||||
usage_hint_text = "base hint"
|
||||
root_agent_usage_hint_text = "base root hint"
|
||||
subagent_usage_hint_text = "base subagent hint"
|
||||
tool_namespace = "base_agents"
|
||||
hide_spawn_agent_metadata = true
|
||||
non_code_mode_only = false
|
||||
|
||||
@@ -10176,6 +10239,7 @@ usage_hint_enabled = false
|
||||
usage_hint_text = "profile hint"
|
||||
root_agent_usage_hint_text = "profile root hint"
|
||||
subagent_usage_hint_text = "profile subagent hint"
|
||||
tool_namespace = "profile_agents"
|
||||
hide_spawn_agent_metadata = false
|
||||
non_code_mode_only = true
|
||||
"#,
|
||||
@@ -10204,6 +10268,10 @@ non_code_mode_only = true
|
||||
config.multi_agent_v2.subagent_usage_hint_text.as_deref(),
|
||||
Some("profile subagent hint")
|
||||
);
|
||||
assert_eq!(
|
||||
config.multi_agent_v2.tool_namespace.as_deref(),
|
||||
Some("profile_agents")
|
||||
);
|
||||
assert!(!config.multi_agent_v2.hide_spawn_agent_metadata);
|
||||
assert!(config.multi_agent_v2.non_code_mode_only);
|
||||
|
||||
@@ -10464,6 +10532,42 @@ default_wait_timeout_ms = 2500
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn multi_agent_v2_rejects_invalid_tool_namespace() -> std::io::Result<()> {
|
||||
for (namespace, expected_message) in [
|
||||
(
|
||||
"bad namespace",
|
||||
"features.multi_agent_v2.tool_namespace must match ^[a-zA-Z0-9_-]+$",
|
||||
),
|
||||
(
|
||||
"functions",
|
||||
"features.multi_agent_v2.tool_namespace uses a reserved namespace: functions",
|
||||
),
|
||||
] {
|
||||
let codex_home = TempDir::new()?;
|
||||
std::fs::write(
|
||||
codex_home.path().join(CONFIG_TOML_FILE),
|
||||
format!(
|
||||
r#"[features.multi_agent_v2]
|
||||
enabled = true
|
||||
tool_namespace = "{namespace}"
|
||||
"#
|
||||
),
|
||||
)?;
|
||||
|
||||
let err = ConfigBuilder::without_managed_config_for_tests()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.fallback_cwd(Some(codex_home.path().to_path_buf()))
|
||||
.build()
|
||||
.await
|
||||
.expect_err("invalid multi_agent_v2 tool namespace should fail");
|
||||
assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput);
|
||||
assert_eq!(err.to_string(), expected_message);
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn multi_agent_v2_session_thread_cap_one_disallows_subagents() -> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
|
||||
@@ -107,6 +107,7 @@ use serde::Serialize;
|
||||
use std::collections::BTreeMap;
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
use std::collections::hash_map::Entry;
|
||||
use std::io::ErrorKind;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
@@ -953,6 +954,7 @@ pub struct MultiAgentV2Config {
|
||||
pub usage_hint_text: Option<String>,
|
||||
pub root_agent_usage_hint_text: Option<String>,
|
||||
pub subagent_usage_hint_text: Option<String>,
|
||||
pub tool_namespace: Option<String>,
|
||||
pub hide_spawn_agent_metadata: bool,
|
||||
pub non_code_mode_only: bool,
|
||||
}
|
||||
@@ -969,6 +971,7 @@ impl Default for MultiAgentV2Config {
|
||||
usage_hint_text: None,
|
||||
root_agent_usage_hint_text: None,
|
||||
subagent_usage_hint_text: None,
|
||||
tool_namespace: None,
|
||||
hide_spawn_agent_metadata: false,
|
||||
non_code_mode_only: false,
|
||||
}
|
||||
@@ -1241,6 +1244,7 @@ impl Config {
|
||||
let plugins_input = self.plugins_config_input();
|
||||
let loaded_plugins = plugins_manager.plugins_for_config(&plugins_input).await;
|
||||
let mut configured_mcp_servers = self.mcp_servers.get().clone();
|
||||
let mut plugin_ids_by_mcp_server_name = HashMap::new();
|
||||
for plugin in loaded_plugins
|
||||
.plugins()
|
||||
.iter()
|
||||
@@ -1253,7 +1257,10 @@ impl Config {
|
||||
self.config_layer_stack.requirements().plugins.as_ref(),
|
||||
);
|
||||
for (name, plugin_server) in plugin_mcp_servers {
|
||||
configured_mcp_servers.entry(name).or_insert(plugin_server);
|
||||
if let Entry::Vacant(entry) = configured_mcp_servers.entry(name.clone()) {
|
||||
entry.insert(plugin_server);
|
||||
plugin_ids_by_mcp_server_name.insert(name, plugin.config_name.clone());
|
||||
}
|
||||
}
|
||||
}
|
||||
if let Some(mcp_requirements) = self.config_layer_stack.requirements().mcp_servers.as_ref()
|
||||
@@ -1263,6 +1270,8 @@ impl Config {
|
||||
// above.
|
||||
filter_mcp_servers_by_requirements(&mut configured_mcp_servers, Some(mcp_requirements));
|
||||
}
|
||||
plugin_ids_by_mcp_server_name
|
||||
.retain(|server_name, _| configured_mcp_servers.contains_key(server_name));
|
||||
|
||||
McpConfig {
|
||||
chatgpt_base_url: self.chatgpt_base_url.clone(),
|
||||
@@ -1290,6 +1299,7 @@ impl Config {
|
||||
ElicitationCapability::default()
|
||||
},
|
||||
configured_mcp_servers,
|
||||
plugin_ids_by_mcp_server_name,
|
||||
plugin_capability_summaries: loaded_plugins.capability_summaries().to_vec(),
|
||||
}
|
||||
}
|
||||
@@ -2178,6 +2188,11 @@ fn resolve_multi_agent_v2_config(
|
||||
.or_else(|| base.and_then(|config| config.subagent_usage_hint_text.as_ref()))
|
||||
.cloned()
|
||||
.or(default.subagent_usage_hint_text);
|
||||
let tool_namespace = profile
|
||||
.and_then(|config| config.tool_namespace.as_ref())
|
||||
.or_else(|| base.and_then(|config| config.tool_namespace.as_ref()))
|
||||
.cloned()
|
||||
.or(default.tool_namespace);
|
||||
let hide_spawn_agent_metadata = profile
|
||||
.and_then(|config| config.hide_spawn_agent_metadata)
|
||||
.or_else(|| base.and_then(|config| config.hide_spawn_agent_metadata))
|
||||
@@ -2196,6 +2211,7 @@ fn resolve_multi_agent_v2_config(
|
||||
usage_hint_text,
|
||||
root_agent_usage_hint_text,
|
||||
subagent_usage_hint_text,
|
||||
tool_namespace,
|
||||
hide_spawn_agent_metadata,
|
||||
non_code_mode_only,
|
||||
}
|
||||
@@ -2290,6 +2306,69 @@ fn validate_multi_agent_v2_wait_timeout(label: &str, value: i64) -> std::io::Res
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn validate_multi_agent_v2_tool_namespace(namespace: Option<&str>) -> std::io::Result<()> {
|
||||
const LABEL: &str = "features.multi_agent_v2.tool_namespace";
|
||||
const MAX_LEN: usize = 64;
|
||||
const RESERVED_RESPONSES_NAMESPACES: &[&str] = &[
|
||||
"api_tool",
|
||||
"browser",
|
||||
"computer",
|
||||
"container",
|
||||
"file_search",
|
||||
"functions",
|
||||
"image_gen",
|
||||
"multi_tool_use",
|
||||
"python",
|
||||
"python_user_visible",
|
||||
"submodel_delegator",
|
||||
"terminal",
|
||||
"tool_search",
|
||||
"web",
|
||||
];
|
||||
|
||||
let Some(namespace) = namespace else {
|
||||
return Ok(());
|
||||
};
|
||||
if namespace.is_empty() {
|
||||
return Err(std::io::Error::new(
|
||||
std::io::ErrorKind::InvalidInput,
|
||||
format!("{LABEL} must not be empty"),
|
||||
));
|
||||
}
|
||||
if namespace.trim() != namespace {
|
||||
return Err(std::io::Error::new(
|
||||
std::io::ErrorKind::InvalidInput,
|
||||
format!("{LABEL} must not have leading or trailing whitespace"),
|
||||
));
|
||||
}
|
||||
if !namespace
|
||||
.bytes()
|
||||
.all(|byte| byte.is_ascii_alphanumeric() || matches!(byte, b'_' | b'-'))
|
||||
{
|
||||
return Err(std::io::Error::new(
|
||||
std::io::ErrorKind::InvalidInput,
|
||||
format!("{LABEL} must match ^[a-zA-Z0-9_-]+$"),
|
||||
));
|
||||
}
|
||||
if namespace.chars().count() > MAX_LEN {
|
||||
return Err(std::io::Error::new(
|
||||
std::io::ErrorKind::InvalidInput,
|
||||
format!("{LABEL} must be at most {MAX_LEN} characters"),
|
||||
));
|
||||
}
|
||||
if namespace == "mcp"
|
||||
|| namespace.starts_with("mcp__")
|
||||
|| RESERVED_RESPONSES_NAMESPACES.contains(&namespace)
|
||||
{
|
||||
return Err(std::io::Error::new(
|
||||
std::io::ErrorKind::InvalidInput,
|
||||
format!("{LABEL} uses a reserved namespace: {namespace}"),
|
||||
));
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
impl Config {
|
||||
#[cfg(test)]
|
||||
async fn load_from_base_config_with_overrides(
|
||||
@@ -2346,8 +2425,10 @@ impl Config {
|
||||
guardian_policy_config_source: _,
|
||||
} = config_layer_stack.requirements().clone();
|
||||
|
||||
let user_instructions = AgentsMdManager::load_global_instructions(Some(&codex_home))
|
||||
.map(|loaded| loaded.contents);
|
||||
let user_instructions =
|
||||
AgentsMdManager::load_global_instructions(LOCAL_FS.as_ref(), Some(&codex_home))
|
||||
.await
|
||||
.map(|loaded| loaded.contents);
|
||||
let mut startup_warnings = config_layer_stack
|
||||
.startup_warnings()
|
||||
.unwrap_or_default()
|
||||
@@ -2911,6 +2992,7 @@ impl Config {
|
||||
"features.multi_agent_v2.default_wait_timeout_ms must be at most features.multi_agent_v2.max_wait_timeout_ms",
|
||||
));
|
||||
}
|
||||
validate_multi_agent_v2_tool_namespace(multi_agent_v2.tool_namespace.as_deref())?;
|
||||
let agent_max_threads_from_config = cfg.agents.as_ref().and_then(|agents| agents.max_threads);
|
||||
let agent_max_threads = if features.enabled(Feature::MultiAgentV2) {
|
||||
if agent_max_threads_from_config.is_some() {
|
||||
|
||||
@@ -2,7 +2,6 @@ use super::*;
|
||||
use base64::Engine;
|
||||
use base64::engine::general_purpose::STANDARD as BASE64_STANDARD;
|
||||
use codex_protocol::AgentPath;
|
||||
use codex_protocol::config_types::ReasoningSummary;
|
||||
use codex_protocol::models::BaseInstructions;
|
||||
use codex_protocol::models::ContentItem;
|
||||
use codex_protocol::models::DEFAULT_IMAGE_DETAIL;
|
||||
@@ -121,7 +120,6 @@ fn developer_msg_with_fragments(texts: &[&str]) -> ResponseItem {
|
||||
fn reference_context_item() -> TurnContextItem {
|
||||
TurnContextItem {
|
||||
turn_id: Some("reference-turn".to_string()),
|
||||
trace_id: None,
|
||||
cwd: PathBuf::from("/tmp/reference-cwd"),
|
||||
current_date: Some("2026-03-23".to_string()),
|
||||
timezone: Some("America/Los_Angeles".to_string()),
|
||||
@@ -135,11 +133,7 @@ fn reference_context_item() -> TurnContextItem {
|
||||
collaboration_mode: None,
|
||||
realtime_active: Some(false),
|
||||
effort: None,
|
||||
summary: ReasoningSummary::Auto,
|
||||
user_instructions: None,
|
||||
developer_instructions: None,
|
||||
final_output_json_schema: None,
|
||||
truncation_policy: Some(codex_protocol::protocol::TruncationPolicy::Tokens(10_000)),
|
||||
summary: codex_protocol::config_types::ReasoningSummary::Auto,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@ use codex_exec_server::LOCAL_FS;
|
||||
use codex_git_utils::GitInfo;
|
||||
use codex_git_utils::GitSha;
|
||||
use codex_git_utils::collect_git_info;
|
||||
use codex_git_utils::get_git_repo_root_with_fs;
|
||||
use codex_git_utils::get_has_changes;
|
||||
use codex_git_utils::git_diff_to_remote;
|
||||
use codex_git_utils::recent_commits;
|
||||
@@ -526,6 +527,20 @@ async fn resolve_root_git_project_for_trust_returns_none_outside_repo() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn get_git_repo_root_with_fs_detects_gitdir_pointer() {
|
||||
let tmp = TempDir::new().expect("tempdir");
|
||||
let proj = tmp.path().join("proj");
|
||||
let nested = proj.join("nested");
|
||||
std::fs::create_dir_all(&nested).unwrap();
|
||||
std::fs::write(proj.join(".git"), "gitdir: /tmp/fake-worktree\n").unwrap();
|
||||
|
||||
assert_eq!(
|
||||
get_git_repo_root_with_fs(LOCAL_FS.as_ref(), &nested.abs()).await,
|
||||
Some(proj.abs())
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn resolve_root_git_project_for_trust_regular_repo_returns_repo_root() {
|
||||
let temp_dir = TempDir::new().expect("Failed to create temp dir");
|
||||
|
||||
+137
-61
@@ -15,21 +15,22 @@ use crate::tasks::RegularTask;
|
||||
use crate::tools::handlers::goal_spec::UPDATE_GOAL_TOOL_NAME;
|
||||
use anyhow::Context;
|
||||
use codex_features::Feature;
|
||||
use codex_otel::GOAL_BLOCKED_METRIC;
|
||||
use codex_otel::GOAL_BUDGET_LIMITED_METRIC;
|
||||
use codex_otel::GOAL_COMPLETED_METRIC;
|
||||
use codex_otel::GOAL_CREATED_METRIC;
|
||||
use codex_otel::GOAL_DURATION_SECONDS_METRIC;
|
||||
use codex_otel::GOAL_RESUMED_METRIC;
|
||||
use codex_otel::GOAL_TOKEN_COUNT_METRIC;
|
||||
use codex_otel::GOAL_USAGE_LIMITED_METRIC;
|
||||
use codex_protocol::config_types::ModeKind;
|
||||
use codex_protocol::models::ContentItem;
|
||||
use codex_protocol::models::ResponseInputItem;
|
||||
use codex_protocol::protocol::Event;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::ThreadGoal;
|
||||
use codex_protocol::protocol::ThreadGoalStatus;
|
||||
use codex_protocol::protocol::ThreadGoalUpdatedEvent;
|
||||
use codex_protocol::protocol::TokenUsage;
|
||||
use codex_protocol::protocol::TurnAbortReason;
|
||||
use codex_protocol::protocol::validate_thread_goal_objective;
|
||||
use codex_rollout::state_db::reconcile_rollout;
|
||||
use codex_thread_store::LocalThreadStore;
|
||||
@@ -153,7 +154,9 @@ pub(crate) enum GoalRuntimeEvent<'a> {
|
||||
MaybeContinueIfIdle,
|
||||
TaskAborted {
|
||||
turn_context: Option<&'a TurnContext>,
|
||||
reason: TurnAbortReason,
|
||||
},
|
||||
UsageLimitReached {
|
||||
turn_context: &'a TurnContext,
|
||||
},
|
||||
ExternalMutationStarting,
|
||||
ExternalSet {
|
||||
@@ -331,8 +334,8 @@ impl Session {
|
||||
/// starts capture the active goal and token baseline, tool completions
|
||||
/// account usage and may inject budget steering, completion accounting
|
||||
/// suppresses that steering, external mutations account best-effort before
|
||||
/// changing state, interrupts pause active goals, thread resumes restore
|
||||
/// runtime state for already-active goals, explicit maybe-continue events
|
||||
/// changing state, thread resumes restore runtime state for already-active
|
||||
/// goals, explicit maybe-continue events
|
||||
/// start idle goal continuation turns, and continuation turns with no counted
|
||||
/// autonomous activity suppress the next automatic continuation until
|
||||
/// user/tool/external activity resets it.
|
||||
@@ -384,12 +387,13 @@ impl Session {
|
||||
self.maybe_continue_goal_if_idle_runtime().await;
|
||||
Ok(())
|
||||
}),
|
||||
GoalRuntimeEvent::TaskAborted {
|
||||
turn_context,
|
||||
reason,
|
||||
} => Box::pin(async move {
|
||||
self.handle_thread_goal_task_abort(turn_context, reason)
|
||||
.await;
|
||||
GoalRuntimeEvent::TaskAborted { turn_context } => Box::pin(async move {
|
||||
self.handle_thread_goal_task_abort(turn_context).await;
|
||||
Ok(())
|
||||
}),
|
||||
GoalRuntimeEvent::UsageLimitReached { turn_context } => Box::pin(async move {
|
||||
self.usage_limit_active_thread_goal_for_turn(turn_context)
|
||||
.await?;
|
||||
Ok(())
|
||||
}),
|
||||
GoalRuntimeEvent::ExternalMutationStarting => Box::pin(async move {
|
||||
@@ -422,6 +426,7 @@ impl Session {
|
||||
|
||||
let state_db = self.require_state_db_for_thread_goals().await?;
|
||||
state_db
|
||||
.thread_goals()
|
||||
.get_thread_goal(self.conversation_id)
|
||||
.await
|
||||
.map(|goal| goal.map(protocol_goal_from_state))
|
||||
@@ -459,10 +464,14 @@ impl Session {
|
||||
let mut replacing_goal = false;
|
||||
let previous_status;
|
||||
let goal = if let Some(objective) = objective.as_deref() {
|
||||
let existing_goal = state_db.get_thread_goal(self.conversation_id).await?;
|
||||
let existing_goal = state_db
|
||||
.thread_goals()
|
||||
.get_thread_goal(self.conversation_id)
|
||||
.await?;
|
||||
previous_status = existing_goal.as_ref().map(|goal| goal.status);
|
||||
if let Some(existing_goal) = existing_goal.as_ref() {
|
||||
state_db
|
||||
.thread_goals()
|
||||
.update_thread_goal(
|
||||
self.conversation_id,
|
||||
codex_state::ThreadGoalUpdate {
|
||||
@@ -482,6 +491,7 @@ impl Session {
|
||||
} else {
|
||||
replacing_goal = true;
|
||||
state_db
|
||||
.thread_goals()
|
||||
.replace_thread_goal(
|
||||
self.conversation_id,
|
||||
objective,
|
||||
@@ -493,11 +503,15 @@ impl Session {
|
||||
.await?
|
||||
}
|
||||
} else {
|
||||
let existing_goal = state_db.get_thread_goal(self.conversation_id).await?;
|
||||
let existing_goal = state_db
|
||||
.thread_goals()
|
||||
.get_thread_goal(self.conversation_id)
|
||||
.await?;
|
||||
previous_status = existing_goal.as_ref().map(|goal| goal.status);
|
||||
let expected_goal_id = existing_goal.map(|goal| goal.goal_id);
|
||||
let status = status.map(state_goal_status_from_protocol);
|
||||
state_db
|
||||
.thread_goals()
|
||||
.update_thread_goal(
|
||||
self.conversation_id,
|
||||
codex_state::ThreadGoalUpdate {
|
||||
@@ -526,6 +540,7 @@ impl Session {
|
||||
if replacing_goal {
|
||||
self.emit_goal_created_metric();
|
||||
}
|
||||
self.emit_goal_resumed_metric_if_status_changed(previous_status_for_goal, goal_status);
|
||||
self.emit_goal_terminal_metrics_if_status_changed(previous_status_for_goal, &goal);
|
||||
let goal = protocol_goal_from_state(goal);
|
||||
*self.goal_runtime.budget_limit_reported_goal_id.lock().await = None;
|
||||
@@ -581,6 +596,7 @@ impl Session {
|
||||
)
|
||||
.await?;
|
||||
let goal = state_db
|
||||
.thread_goals()
|
||||
.insert_thread_goal(
|
||||
self.conversation_id,
|
||||
objective,
|
||||
@@ -641,6 +657,7 @@ impl Session {
|
||||
let previous_status = previous_goal
|
||||
.as_ref()
|
||||
.and_then(|previous_goal| (!replaced_existing_goal).then_some(previous_goal.status));
|
||||
self.emit_goal_resumed_metric_if_status_changed(previous_status, goal.status);
|
||||
self.emit_goal_terminal_metrics_if_status_changed(previous_status, &goal);
|
||||
let goal_for_steering = objective_changed.then(|| protocol_goal_from_state(goal.clone()));
|
||||
let goal_id = goal.goal_id;
|
||||
@@ -669,7 +686,10 @@ impl Session {
|
||||
self.clear_stopped_thread_goal_runtime_state().await;
|
||||
}
|
||||
}
|
||||
codex_state::ThreadGoalStatus::Paused | codex_state::ThreadGoalStatus::Complete => {
|
||||
codex_state::ThreadGoalStatus::Paused
|
||||
| codex_state::ThreadGoalStatus::Blocked
|
||||
| codex_state::ThreadGoalStatus::UsageLimited
|
||||
| codex_state::ThreadGoalStatus::Complete => {
|
||||
self.clear_stopped_thread_goal_runtime_state().await;
|
||||
}
|
||||
}
|
||||
@@ -723,6 +743,31 @@ impl Session {
|
||||
.counter(GOAL_CREATED_METRIC, /*inc*/ 1, &[]);
|
||||
}
|
||||
|
||||
fn emit_goal_resumed_metric(&self) {
|
||||
self.services
|
||||
.session_telemetry
|
||||
.counter(GOAL_RESUMED_METRIC, /*inc*/ 1, &[]);
|
||||
}
|
||||
|
||||
fn emit_goal_resumed_metric_if_status_changed(
|
||||
&self,
|
||||
previous_status: Option<codex_state::ThreadGoalStatus>,
|
||||
goal_status: codex_state::ThreadGoalStatus,
|
||||
) {
|
||||
if goal_status == codex_state::ThreadGoalStatus::Active
|
||||
&& matches!(
|
||||
previous_status,
|
||||
Some(
|
||||
codex_state::ThreadGoalStatus::Paused
|
||||
| codex_state::ThreadGoalStatus::Blocked
|
||||
| codex_state::ThreadGoalStatus::UsageLimited
|
||||
)
|
||||
)
|
||||
{
|
||||
self.emit_goal_resumed_metric();
|
||||
}
|
||||
}
|
||||
|
||||
fn emit_goal_terminal_metrics_if_status_changed(
|
||||
&self,
|
||||
previous_status: Option<codex_state::ThreadGoalStatus>,
|
||||
@@ -733,6 +778,8 @@ impl Session {
|
||||
}
|
||||
|
||||
let counter = match goal.status {
|
||||
codex_state::ThreadGoalStatus::Blocked => GOAL_BLOCKED_METRIC,
|
||||
codex_state::ThreadGoalStatus::UsageLimited => GOAL_USAGE_LIMITED_METRIC,
|
||||
codex_state::ThreadGoalStatus::BudgetLimited => GOAL_BUDGET_LIMITED_METRIC,
|
||||
codex_state::ThreadGoalStatus::Complete => GOAL_COMPLETED_METRIC,
|
||||
codex_state::ThreadGoalStatus::Active | codex_state::ThreadGoalStatus::Paused => {
|
||||
@@ -760,7 +807,10 @@ impl Session {
|
||||
state_db: &StateDbHandle,
|
||||
expected_goal_id: Option<&str>,
|
||||
) -> anyhow::Result<Option<codex_state::ThreadGoalStatus>> {
|
||||
let goal = state_db.get_thread_goal(self.conversation_id).await?;
|
||||
let goal = state_db
|
||||
.thread_goals()
|
||||
.get_thread_goal(self.conversation_id)
|
||||
.await?;
|
||||
Ok(goal.and_then(|goal| {
|
||||
expected_goal_id
|
||||
.is_none_or(|expected_goal_id| goal.goal_id == expected_goal_id)
|
||||
@@ -801,7 +851,11 @@ impl Session {
|
||||
return;
|
||||
}
|
||||
};
|
||||
match state_db.get_thread_goal(self.conversation_id).await {
|
||||
match state_db
|
||||
.thread_goals()
|
||||
.get_thread_goal(self.conversation_id)
|
||||
.await
|
||||
{
|
||||
Ok(Some(goal))
|
||||
if matches!(
|
||||
goal.status,
|
||||
@@ -886,11 +940,7 @@ impl Session {
|
||||
}
|
||||
}
|
||||
|
||||
async fn handle_thread_goal_task_abort(
|
||||
&self,
|
||||
turn_context: Option<&TurnContext>,
|
||||
reason: TurnAbortReason,
|
||||
) {
|
||||
async fn handle_thread_goal_task_abort(&self, turn_context: Option<&TurnContext>) {
|
||||
if let Some(turn_context) = turn_context {
|
||||
self.take_thread_goal_continuation_turn(&turn_context.sub_id)
|
||||
.await;
|
||||
@@ -913,12 +963,6 @@ impl Session {
|
||||
accounting.turn = None;
|
||||
}
|
||||
}
|
||||
|
||||
if reason == TurnAbortReason::Interrupted
|
||||
&& let Err(err) = self.pause_active_thread_goal_for_interrupt().await
|
||||
{
|
||||
tracing::warn!("failed to pause active thread goal after interrupt: {err}");
|
||||
}
|
||||
}
|
||||
|
||||
async fn account_thread_goal_progress(
|
||||
@@ -963,6 +1007,7 @@ impl Session {
|
||||
.current_goal_status_for_metrics(&state_db, expected_goal_id.as_deref())
|
||||
.await?;
|
||||
let outcome = state_db
|
||||
.thread_goals()
|
||||
.account_thread_goal_usage(
|
||||
self.conversation_id,
|
||||
time_delta_seconds,
|
||||
@@ -985,6 +1030,8 @@ impl Session {
|
||||
matches!(budget_limit_steering, BudgetLimitSteering::Suppressed)
|
||||
}
|
||||
codex_state::ThreadGoalStatus::Paused
|
||||
| codex_state::ThreadGoalStatus::Blocked
|
||||
| codex_state::ThreadGoalStatus::UsageLimited
|
||||
| codex_state::ThreadGoalStatus::Complete => true,
|
||||
};
|
||||
{
|
||||
@@ -1085,6 +1132,7 @@ impl Session {
|
||||
.await?;
|
||||
|
||||
match state_db
|
||||
.thread_goals()
|
||||
.account_thread_goal_usage(
|
||||
self.conversation_id,
|
||||
time_delta_seconds,
|
||||
@@ -1122,8 +1170,11 @@ impl Session {
|
||||
}
|
||||
}
|
||||
|
||||
async fn pause_active_thread_goal_for_interrupt(&self) -> anyhow::Result<()> {
|
||||
if should_ignore_goal_for_mode(self.collaboration_mode().await.mode) {
|
||||
async fn usage_limit_active_thread_goal_for_turn(
|
||||
&self,
|
||||
turn_context: &TurnContext,
|
||||
) -> anyhow::Result<()> {
|
||||
if should_ignore_goal_for_mode(turn_context.collaboration_mode.mode) {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
@@ -1140,34 +1191,34 @@ impl Session {
|
||||
let Some(state_db) = self.state_db_for_thread_goals().await? else {
|
||||
return Ok(());
|
||||
};
|
||||
self.account_thread_goal_wall_clock_usage(
|
||||
&state_db,
|
||||
codex_state::ThreadGoalAccountingMode::ActiveStatusOnly,
|
||||
self.account_thread_goal_progress(
|
||||
turn_context,
|
||||
BudgetLimitSteering::Suppressed,
|
||||
TerminalMetricEmission::Emit,
|
||||
)
|
||||
.await?;
|
||||
let previous_status = self
|
||||
.current_goal_status_for_metrics(&state_db, /*expected_goal_id*/ None)
|
||||
.await?;
|
||||
let Some(goal) = state_db
|
||||
.pause_active_thread_goal(self.conversation_id)
|
||||
.thread_goals()
|
||||
.usage_limit_active_thread_goal(self.conversation_id)
|
||||
.await?
|
||||
else {
|
||||
return Ok(());
|
||||
};
|
||||
self.emit_goal_terminal_metrics_if_status_changed(previous_status, &goal);
|
||||
let goal = protocol_goal_from_state(goal);
|
||||
*self.goal_runtime.budget_limit_reported_goal_id.lock().await = None;
|
||||
self.goal_runtime
|
||||
.accounting
|
||||
.lock()
|
||||
.await
|
||||
.wall_clock
|
||||
.clear_active_goal();
|
||||
self.send_event_raw(Event {
|
||||
id: uuid::Uuid::new_v4().to_string(),
|
||||
msg: EventMsg::ThreadGoalUpdated(ThreadGoalUpdatedEvent {
|
||||
self.clear_active_goal_accounting(turn_context).await;
|
||||
self.send_event(
|
||||
turn_context,
|
||||
EventMsg::ThreadGoalUpdated(ThreadGoalUpdatedEvent {
|
||||
thread_id: self.conversation_id,
|
||||
turn_id: None,
|
||||
turn_id: Some(turn_context.sub_id.clone()),
|
||||
goal,
|
||||
}),
|
||||
})
|
||||
)
|
||||
.await;
|
||||
Ok(())
|
||||
}
|
||||
@@ -1192,7 +1243,11 @@ impl Session {
|
||||
let Some(state_db) = self.state_db_for_thread_goals().await? else {
|
||||
return Ok(());
|
||||
};
|
||||
let Some(goal) = state_db.get_thread_goal(self.conversation_id).await? else {
|
||||
let Some(goal) = state_db
|
||||
.thread_goals()
|
||||
.get_thread_goal(self.conversation_id)
|
||||
.await?
|
||||
else {
|
||||
self.clear_stopped_thread_goal_runtime_state().await;
|
||||
return Ok(());
|
||||
};
|
||||
@@ -1204,8 +1259,11 @@ impl Session {
|
||||
.await
|
||||
.wall_clock
|
||||
.mark_active_goal(goal.goal_id);
|
||||
self.emit_goal_resumed_metric();
|
||||
}
|
||||
codex_state::ThreadGoalStatus::Paused
|
||||
| codex_state::ThreadGoalStatus::Blocked
|
||||
| codex_state::ThreadGoalStatus::UsageLimited
|
||||
| codex_state::ThreadGoalStatus::BudgetLimited
|
||||
| codex_state::ThreadGoalStatus::Complete => {
|
||||
self.clear_stopped_thread_goal_runtime_state().await;
|
||||
@@ -1237,7 +1295,11 @@ impl Session {
|
||||
Arc::clone(&active_turn.turn_state)
|
||||
};
|
||||
let goal_is_current = match self.state_db_for_thread_goals().await {
|
||||
Ok(Some(state_db)) => match state_db.get_thread_goal(self.conversation_id).await {
|
||||
Ok(Some(state_db)) => match state_db
|
||||
.thread_goals()
|
||||
.get_thread_goal(self.conversation_id)
|
||||
.await
|
||||
{
|
||||
Ok(Some(goal))
|
||||
if goal.goal_id == candidate.goal_id
|
||||
&& goal.status == codex_state::ThreadGoalStatus::Active =>
|
||||
@@ -1269,12 +1331,9 @@ impl Session {
|
||||
.await;
|
||||
return;
|
||||
}
|
||||
{
|
||||
let mut turn_state = turn_state.lock().await;
|
||||
for item in candidate.items {
|
||||
turn_state.push_pending_input(item);
|
||||
}
|
||||
}
|
||||
self.input_queue
|
||||
.extend_pending_input_for_turn_state(turn_state.as_ref(), candidate.items)
|
||||
.await;
|
||||
|
||||
let turn_context = self
|
||||
.new_default_turn_with_sub_id(uuid::Uuid::new_v4().to_string())
|
||||
@@ -1312,11 +1371,15 @@ impl Session {
|
||||
tracing::debug!("skipping active goal continuation because a turn is already active");
|
||||
return None;
|
||||
}
|
||||
if self.has_queued_response_items_for_next_turn().await {
|
||||
if self
|
||||
.input_queue
|
||||
.has_queued_response_items_for_next_turn()
|
||||
.await
|
||||
{
|
||||
tracing::debug!("skipping active goal continuation because queued input exists");
|
||||
return None;
|
||||
}
|
||||
if self.has_trigger_turn_mailbox_items().await {
|
||||
if self.input_queue.has_trigger_turn_mailbox_items().await {
|
||||
tracing::debug!(
|
||||
"skipping active goal continuation because trigger-turn mailbox input is pending"
|
||||
);
|
||||
@@ -1333,7 +1396,11 @@ impl Session {
|
||||
return None;
|
||||
}
|
||||
};
|
||||
let goal = match state_db.get_thread_goal(self.conversation_id).await {
|
||||
let goal = match state_db
|
||||
.thread_goals()
|
||||
.get_thread_goal(self.conversation_id)
|
||||
.await
|
||||
{
|
||||
Ok(Some(goal)) => goal,
|
||||
Ok(None) => {
|
||||
tracing::debug!("skipping active goal continuation because no goal is set");
|
||||
@@ -1349,8 +1416,11 @@ impl Session {
|
||||
return None;
|
||||
}
|
||||
if self.active_turn.lock().await.is_some()
|
||||
|| self.has_queued_response_items_for_next_turn().await
|
||||
|| self.has_trigger_turn_mailbox_items().await
|
||||
|| self
|
||||
.input_queue
|
||||
.has_queued_response_items_for_next_turn()
|
||||
.await
|
||||
|| self.input_queue.has_trigger_turn_mailbox_items().await
|
||||
{
|
||||
tracing::debug!("skipping active goal continuation because pending work appeared");
|
||||
return None;
|
||||
@@ -1553,6 +1623,8 @@ pub(crate) fn protocol_goal_status_from_state(
|
||||
match status {
|
||||
codex_state::ThreadGoalStatus::Active => ThreadGoalStatus::Active,
|
||||
codex_state::ThreadGoalStatus::Paused => ThreadGoalStatus::Paused,
|
||||
codex_state::ThreadGoalStatus::Blocked => ThreadGoalStatus::Blocked,
|
||||
codex_state::ThreadGoalStatus::UsageLimited => ThreadGoalStatus::UsageLimited,
|
||||
codex_state::ThreadGoalStatus::BudgetLimited => ThreadGoalStatus::BudgetLimited,
|
||||
codex_state::ThreadGoalStatus::Complete => ThreadGoalStatus::Complete,
|
||||
}
|
||||
@@ -1564,6 +1636,8 @@ pub(crate) fn state_goal_status_from_protocol(
|
||||
match status {
|
||||
ThreadGoalStatus::Active => codex_state::ThreadGoalStatus::Active,
|
||||
ThreadGoalStatus::Paused => codex_state::ThreadGoalStatus::Paused,
|
||||
ThreadGoalStatus::Blocked => codex_state::ThreadGoalStatus::Blocked,
|
||||
ThreadGoalStatus::UsageLimited => codex_state::ThreadGoalStatus::UsageLimited,
|
||||
ThreadGoalStatus::BudgetLimited => codex_state::ThreadGoalStatus::BudgetLimited,
|
||||
ThreadGoalStatus::Complete => codex_state::ThreadGoalStatus::Complete,
|
||||
}
|
||||
@@ -1642,7 +1716,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn continuation_prompt_only_tells_model_to_update_goal_when_complete() {
|
||||
fn continuation_prompt_allows_complete_and_strict_blocked_updates() {
|
||||
let prompt = continuation_prompt(&ThreadGoal {
|
||||
thread_id: ThreadId::new(),
|
||||
objective: "finish the stack".to_string(),
|
||||
@@ -1659,9 +1733,11 @@ mod tests {
|
||||
assert!(prompt.contains("<objective>\nfinish the stack\n</objective>"));
|
||||
assert!(prompt.contains("Token budget: 10000"));
|
||||
assert!(prompt.contains("call update_goal with status \"complete\""));
|
||||
assert!(!prompt.contains(
|
||||
"explain the blocker or next required input to the user and wait for new input"
|
||||
));
|
||||
assert!(prompt.contains("status \"blocked\""));
|
||||
assert!(prompt.contains("at least three consecutive goal turns"));
|
||||
assert!(prompt.contains("same blocking condition"));
|
||||
assert!(prompt.contains("original/user-triggered turn"));
|
||||
assert!(prompt.contains("truly at an impasse"));
|
||||
assert!(!prompt.contains("budgetLimited"));
|
||||
assert!(!prompt.contains("status \"paused\""));
|
||||
}
|
||||
|
||||
@@ -90,13 +90,11 @@ pub(crate) use skills::SkillsManager;
|
||||
pub(crate) use skills::build_available_skills;
|
||||
pub(crate) use skills::build_skill_injections;
|
||||
pub(crate) use skills::build_skill_name_counts;
|
||||
pub(crate) use skills::collect_env_var_dependencies;
|
||||
pub(crate) use skills::collect_explicit_skill_mentions;
|
||||
pub(crate) use skills::default_skill_metadata_budget;
|
||||
pub(crate) use skills::injection;
|
||||
pub(crate) use skills::manager;
|
||||
pub(crate) use skills::maybe_emit_implicit_skill_invocation;
|
||||
pub(crate) use skills::resolve_skill_dependencies_for_turn;
|
||||
pub(crate) use skills::skills_load_input_from_config;
|
||||
mod stream_events_utils;
|
||||
pub mod test_support;
|
||||
|
||||
@@ -974,6 +974,7 @@ pub(crate) struct McpToolApprovalMetadata {
|
||||
connector_id: Option<String>,
|
||||
connector_name: Option<String>,
|
||||
connector_description: Option<String>,
|
||||
plugin_id: Option<String>,
|
||||
tool_title: Option<String>,
|
||||
tool_description: Option<String>,
|
||||
mcp_app_resource_uri: Option<String>,
|
||||
@@ -983,6 +984,7 @@ pub(crate) struct McpToolApprovalMetadata {
|
||||
|
||||
const MCP_TOOL_OPENAI_OUTPUT_TEMPLATE_META_KEY: &str = "openai/outputTemplate";
|
||||
const MCP_TOOL_UI_RESOURCE_URI_META_KEY: &str = "ui/resourceUri";
|
||||
const MCP_TOOL_PLUGIN_ID_META_KEY: &str = "plugin_id";
|
||||
const MCP_TOOL_THREAD_ID_META_KEY: &str = "threadId";
|
||||
|
||||
async fn custom_mcp_tool_approval_mode(
|
||||
@@ -1068,6 +1070,12 @@ fn build_mcp_tool_call_request_meta(
|
||||
serde_json::Value::Object(codex_apps_meta),
|
||||
);
|
||||
}
|
||||
if let Some(plugin_id) = metadata.and_then(|metadata| metadata.plugin_id.as_ref()) {
|
||||
request_meta.insert(
|
||||
MCP_TOOL_PLUGIN_ID_META_KEY.to_string(),
|
||||
serde_json::Value::String(plugin_id.clone()),
|
||||
);
|
||||
}
|
||||
|
||||
(!request_meta.is_empty()).then_some(serde_json::Value::Object(request_meta))
|
||||
}
|
||||
@@ -1480,13 +1488,11 @@ pub(crate) async fn lookup_mcp_tool_metadata(
|
||||
server: &str,
|
||||
tool_name: &str,
|
||||
) -> Option<McpToolApprovalMetadata> {
|
||||
let tools = sess
|
||||
.services
|
||||
.mcp_connection_manager
|
||||
.read()
|
||||
.await
|
||||
.list_all_tools()
|
||||
.await;
|
||||
let manager = sess.services.mcp_connection_manager.read().await;
|
||||
let plugin_id = manager
|
||||
.plugin_id_for_mcp_server_name(server)
|
||||
.map(str::to_string);
|
||||
let tools = manager.list_all_tools().await;
|
||||
let tool_info = tools
|
||||
.into_iter()
|
||||
.find(|tool_info| tool_info.server_name == server && tool_info.tool.name == tool_name)?;
|
||||
@@ -1519,6 +1525,7 @@ pub(crate) async fn lookup_mcp_tool_metadata(
|
||||
connector_id: tool_info.connector_id,
|
||||
connector_name: tool_info.connector_name,
|
||||
connector_description,
|
||||
plugin_id,
|
||||
tool_title: tool_info.tool.title,
|
||||
tool_description: tool_info.tool.description.map(std::borrow::Cow::into_owned),
|
||||
mcp_app_resource_uri: get_mcp_app_resource_uri(tool_info.tool.meta.as_deref()),
|
||||
|
||||
@@ -76,6 +76,7 @@ fn approval_metadata(
|
||||
connector_id: connector_id.map(str::to_string),
|
||||
connector_name: connector_name.map(str::to_string),
|
||||
connector_description: connector_description.map(str::to_string),
|
||||
plugin_id: None,
|
||||
tool_title: tool_title.map(str::to_string),
|
||||
tool_description: tool_description.map(str::to_string),
|
||||
mcp_app_resource_uri: None,
|
||||
@@ -1099,6 +1100,29 @@ async fn mcp_tool_call_request_meta_includes_turn_started_at_unix_ms() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn plugin_mcp_tool_call_request_meta_includes_plugin_id() {
|
||||
let (_, turn_context) = make_session_and_context().await;
|
||||
let expected_turn_metadata = turn_context
|
||||
.turn_metadata_state
|
||||
.current_meta_value_for_mcp_request(mcp_turn_metadata_context(&turn_context))
|
||||
.expect("turn metadata");
|
||||
let mut metadata = approval_metadata(
|
||||
/*connector_id*/ None, /*connector_name*/ None,
|
||||
/*connector_description*/ None, /*tool_title*/ None,
|
||||
/*tool_description*/ None,
|
||||
);
|
||||
metadata.plugin_id = Some("sample@test".to_string());
|
||||
|
||||
assert_eq!(
|
||||
build_mcp_tool_call_request_meta(&turn_context, "sample", "call-plugin", Some(&metadata),),
|
||||
Some(serde_json::json!({
|
||||
crate::X_CODEX_TURN_METADATA_HEADER: expected_turn_metadata,
|
||||
MCP_TOOL_PLUGIN_ID_META_KEY: "sample@test",
|
||||
}))
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn codex_apps_tool_call_request_meta_includes_turn_metadata_and_codex_apps_meta() {
|
||||
let (_, turn_context) = make_session_and_context().await;
|
||||
@@ -1111,6 +1135,7 @@ async fn codex_apps_tool_call_request_meta_includes_turn_metadata_and_codex_apps
|
||||
connector_id: Some("calendar".to_string()),
|
||||
connector_name: Some("Calendar".to_string()),
|
||||
connector_description: Some("Manage events".to_string()),
|
||||
plugin_id: None,
|
||||
tool_title: Some("Create Event".to_string()),
|
||||
tool_description: Some("Create a calendar event.".to_string()),
|
||||
mcp_app_resource_uri: None,
|
||||
@@ -1573,6 +1598,7 @@ fn guardian_mcp_review_request_includes_annotations_when_present() {
|
||||
connector_id: None,
|
||||
connector_name: None,
|
||||
connector_description: None,
|
||||
plugin_id: None,
|
||||
tool_title: None,
|
||||
tool_description: None,
|
||||
mcp_app_resource_uri: None,
|
||||
@@ -2290,6 +2316,7 @@ async fn approve_mode_skips_when_annotations_do_not_require_approval() {
|
||||
connector_id: None,
|
||||
connector_name: None,
|
||||
connector_description: None,
|
||||
plugin_id: None,
|
||||
tool_title: Some("Read Only Tool".to_string()),
|
||||
tool_description: None,
|
||||
mcp_app_resource_uri: None,
|
||||
@@ -2363,6 +2390,7 @@ async fn guardian_mode_skips_auto_when_annotations_do_not_require_approval() {
|
||||
connector_id: None,
|
||||
connector_name: None,
|
||||
connector_description: None,
|
||||
plugin_id: None,
|
||||
tool_title: Some("Read Only Tool".to_string()),
|
||||
tool_description: None,
|
||||
mcp_app_resource_uri: None,
|
||||
@@ -2419,6 +2447,7 @@ async fn permission_request_hook_allows_mcp_tool_call() {
|
||||
connector_id: None,
|
||||
connector_name: None,
|
||||
connector_description: None,
|
||||
plugin_id: None,
|
||||
tool_title: Some("Create entities".to_string()),
|
||||
tool_description: None,
|
||||
mcp_app_resource_uri: None,
|
||||
@@ -2554,6 +2583,7 @@ async fn permission_request_hook_runs_after_remembered_mcp_approval() {
|
||||
connector_id: None,
|
||||
connector_name: None,
|
||||
connector_description: None,
|
||||
plugin_id: None,
|
||||
tool_title: Some("Create entities".to_string()),
|
||||
tool_description: None,
|
||||
mcp_app_resource_uri: None,
|
||||
@@ -2640,6 +2670,7 @@ async fn guardian_mode_mcp_denial_returns_rationale_message() {
|
||||
connector_id: None,
|
||||
connector_name: None,
|
||||
connector_description: None,
|
||||
plugin_id: None,
|
||||
tool_title: Some("Dangerous Tool".to_string()),
|
||||
tool_description: Some("Reads calendar data.".to_string()),
|
||||
mcp_app_resource_uri: None,
|
||||
@@ -2693,6 +2724,7 @@ async fn prompt_mode_waits_for_approval_when_annotations_do_not_require_approval
|
||||
connector_id: None,
|
||||
connector_name: None,
|
||||
connector_description: None,
|
||||
plugin_id: None,
|
||||
tool_title: Some("Read Only Tool".to_string()),
|
||||
tool_description: None,
|
||||
mcp_app_resource_uri: None,
|
||||
@@ -2772,6 +2804,7 @@ async fn approve_mode_skips_arc_interrupt_for_model() {
|
||||
connector_id: Some("calendar".to_string()),
|
||||
connector_name: Some("Calendar".to_string()),
|
||||
connector_description: Some("Manage events".to_string()),
|
||||
plugin_id: None,
|
||||
tool_title: Some("Dangerous Tool".to_string()),
|
||||
tool_description: Some("Performs a risky action.".to_string()),
|
||||
mcp_app_resource_uri: None,
|
||||
@@ -2839,6 +2872,7 @@ async fn custom_approve_mode_skips_arc_interrupt_for_model() {
|
||||
connector_id: None,
|
||||
connector_name: None,
|
||||
connector_description: None,
|
||||
plugin_id: None,
|
||||
tool_title: Some("Dangerous Tool".to_string()),
|
||||
tool_description: Some("Performs a risky action.".to_string()),
|
||||
mcp_app_resource_uri: None,
|
||||
@@ -2906,6 +2940,7 @@ async fn approve_mode_skips_arc_interrupt_without_annotations() {
|
||||
connector_id: Some("calendar".to_string()),
|
||||
connector_name: Some("Calendar".to_string()),
|
||||
connector_description: Some("Manage events".to_string()),
|
||||
plugin_id: None,
|
||||
tool_title: Some("Dangerous Tool".to_string()),
|
||||
tool_description: Some("Performs a risky action.".to_string()),
|
||||
mcp_app_resource_uri: None,
|
||||
@@ -2978,6 +3013,7 @@ async fn full_access_mode_skips_arc_monitor_for_all_approval_modes() {
|
||||
connector_id: Some("calendar".to_string()),
|
||||
connector_name: Some("Calendar".to_string()),
|
||||
connector_description: Some("Manage events".to_string()),
|
||||
plugin_id: None,
|
||||
tool_title: Some("Dangerous Tool".to_string()),
|
||||
tool_description: Some("Performs a risky action.".to_string()),
|
||||
mcp_app_resource_uri: None,
|
||||
@@ -3046,6 +3082,7 @@ async fn approve_mode_skips_arc_and_guardian_in_every_permission_mode() {
|
||||
connector_id: Some("calendar".to_string()),
|
||||
connector_name: Some("Calendar".to_string()),
|
||||
connector_description: Some("Manage events".to_string()),
|
||||
plugin_id: None,
|
||||
tool_title: Some("Dangerous Tool".to_string()),
|
||||
tool_description: Some("Performs a risky action.".to_string()),
|
||||
mcp_app_resource_uri: None,
|
||||
|
||||
@@ -263,7 +263,9 @@ pub async fn inter_agent_communication(
|
||||
communication: InterAgentCommunication,
|
||||
) {
|
||||
let trigger_turn = communication.trigger_turn;
|
||||
sess.enqueue_mailbox_communication(communication);
|
||||
sess.input_queue
|
||||
.enqueue_mailbox_communication(communication)
|
||||
.await;
|
||||
if trigger_turn {
|
||||
sess.maybe_start_turn_for_pending_work_with_sub_id(sub_id)
|
||||
.await;
|
||||
@@ -582,12 +584,14 @@ async fn shutdown_session_runtime(sess: &Arc<Session>) {
|
||||
sess.guardian_review_session.shutdown().await;
|
||||
}
|
||||
|
||||
fn emit_thread_stop_lifecycle(sess: &Session) {
|
||||
async fn emit_thread_stop_lifecycle(sess: &Session) {
|
||||
for contributor in sess.services.extensions.thread_lifecycle_contributors() {
|
||||
contributor.on_thread_stop(codex_extension_api::ThreadStopInput {
|
||||
session_store: &sess.services.session_extension_data,
|
||||
thread_store: &sess.services.thread_extension_data,
|
||||
});
|
||||
contributor
|
||||
.on_thread_stop(codex_extension_api::ThreadStopInput {
|
||||
session_store: &sess.services.session_extension_data,
|
||||
thread_store: &sess.services.thread_extension_data,
|
||||
})
|
||||
.await;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -606,7 +610,7 @@ pub async fn shutdown(sess: &Arc<Session>, sub_id: String) -> bool {
|
||||
&[],
|
||||
);
|
||||
|
||||
emit_thread_stop_lifecycle(sess.as_ref());
|
||||
emit_thread_stop_lifecycle(sess.as_ref()).await;
|
||||
|
||||
// Gracefully flush and shutdown thread persistence on session end so tests
|
||||
// that inspect durable state do not race with the background writer.
|
||||
@@ -859,7 +863,7 @@ pub(super) async fn submission_loop(
|
||||
// explicit shutdown op, still run session teardown.
|
||||
if !shutdown_received {
|
||||
shutdown_session_runtime(&sess).await;
|
||||
emit_thread_stop_lifecycle(sess.as_ref());
|
||||
emit_thread_stop_lifecycle(sess.as_ref()).await;
|
||||
}
|
||||
debug!("Agent loop exited");
|
||||
}
|
||||
@@ -901,7 +905,9 @@ Approved action:
|
||||
}];
|
||||
|
||||
if let Err(items) = sess.inject_response_items(items).await {
|
||||
sess.queue_response_items_for_next_turn(items).await;
|
||||
sess.input_queue
|
||||
.queue_response_items_for_next_turn(items)
|
||||
.await;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,392 @@
|
||||
use crate::state::ActiveTurn;
|
||||
use crate::state::MailboxDeliveryPhase;
|
||||
use crate::state::TurnState;
|
||||
use codex_protocol::models::ResponseInputItem;
|
||||
use codex_protocol::protocol::InterAgentCommunication;
|
||||
use std::collections::VecDeque;
|
||||
use std::sync::Arc;
|
||||
use tokio::sync::Mutex;
|
||||
use tokio::sync::watch;
|
||||
|
||||
/// Turn-local pending input storage owned by the input queue flow.
|
||||
#[derive(Default)]
|
||||
pub(crate) struct TurnInputQueue {
|
||||
items: Vec<ResponseInputItem>,
|
||||
}
|
||||
|
||||
/// Session-scoped pending input storage and active-turn mailbox delivery coordination.
|
||||
pub(crate) struct InputQueue {
|
||||
mailbox_tx: watch::Sender<()>,
|
||||
mailbox_pending_mails: Mutex<VecDeque<InterAgentCommunication>>,
|
||||
|
||||
idle_pending_input: Mutex<Vec<ResponseInputItem>>,
|
||||
}
|
||||
|
||||
impl InputQueue {
|
||||
pub(crate) fn new() -> Self {
|
||||
let (mailbox_tx, _) = watch::channel(());
|
||||
Self {
|
||||
mailbox_tx,
|
||||
mailbox_pending_mails: Mutex::new(VecDeque::new()),
|
||||
idle_pending_input: Mutex::new(Vec::new()),
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) async fn subscribe_mailbox(&self) -> watch::Receiver<()> {
|
||||
let mut mailbox_rx = self.mailbox_tx.subscribe();
|
||||
if self.has_pending_mailbox_items().await {
|
||||
mailbox_rx.mark_changed();
|
||||
}
|
||||
mailbox_rx
|
||||
}
|
||||
|
||||
pub(crate) async fn enqueue_mailbox_communication(
|
||||
&self,
|
||||
communication: InterAgentCommunication,
|
||||
) {
|
||||
self.mailbox_pending_mails
|
||||
.lock()
|
||||
.await
|
||||
.push_back(communication);
|
||||
self.mailbox_tx.send_replace(());
|
||||
}
|
||||
|
||||
pub(crate) async fn has_pending_mailbox_items(&self) -> bool {
|
||||
!self.mailbox_pending_mails.lock().await.is_empty()
|
||||
}
|
||||
|
||||
pub(crate) async fn has_trigger_turn_mailbox_items(&self) -> bool {
|
||||
self.mailbox_pending_mails
|
||||
.lock()
|
||||
.await
|
||||
.iter()
|
||||
.any(|mail| mail.trigger_turn)
|
||||
}
|
||||
|
||||
pub(crate) async fn drain_mailbox_input_items(&self) -> Vec<ResponseInputItem> {
|
||||
self.mailbox_pending_mails
|
||||
.lock()
|
||||
.await
|
||||
.drain(..)
|
||||
.map(|mail| mail.to_response_input_item())
|
||||
.collect()
|
||||
}
|
||||
|
||||
pub(crate) async fn queue_response_items_for_next_turn(&self, items: Vec<ResponseInputItem>) {
|
||||
if items.is_empty() {
|
||||
return;
|
||||
}
|
||||
|
||||
self.idle_pending_input.lock().await.extend(items);
|
||||
}
|
||||
|
||||
pub(crate) async fn take_queued_response_items_for_next_turn(&self) -> Vec<ResponseInputItem> {
|
||||
std::mem::take(&mut *self.idle_pending_input.lock().await)
|
||||
}
|
||||
|
||||
pub(crate) async fn has_queued_response_items_for_next_turn(&self) -> bool {
|
||||
!self.idle_pending_input.lock().await.is_empty()
|
||||
}
|
||||
|
||||
pub(crate) async fn turn_state_for_sub_id(
|
||||
&self,
|
||||
active_turn: &Mutex<Option<ActiveTurn>>,
|
||||
sub_id: &str,
|
||||
) -> Option<Arc<Mutex<TurnState>>> {
|
||||
let active = active_turn.lock().await;
|
||||
active.as_ref().and_then(|active_turn| {
|
||||
active_turn
|
||||
.tasks
|
||||
.contains_key(sub_id)
|
||||
.then(|| Arc::clone(&active_turn.turn_state))
|
||||
})
|
||||
}
|
||||
|
||||
/// Clear any pending waiters and input buffered for the current turn.
|
||||
pub(crate) async fn clear_pending(&self, active_turn: &ActiveTurn) {
|
||||
let mut turn_state = active_turn.turn_state.lock().await;
|
||||
turn_state.clear_pending_waiters();
|
||||
turn_state.pending_input.items.clear();
|
||||
}
|
||||
|
||||
pub(crate) async fn defer_mailbox_delivery_to_next_turn(
|
||||
&self,
|
||||
active_turn: &Mutex<Option<ActiveTurn>>,
|
||||
sub_id: &str,
|
||||
) {
|
||||
let turn_state = self.turn_state_for_sub_id(active_turn, sub_id).await;
|
||||
let Some(turn_state) = turn_state else {
|
||||
return;
|
||||
};
|
||||
let mut turn_state = turn_state.lock().await;
|
||||
if !turn_state.pending_input.items.is_empty() {
|
||||
return;
|
||||
}
|
||||
turn_state.set_mailbox_delivery_phase(MailboxDeliveryPhase::NextTurn);
|
||||
}
|
||||
|
||||
pub(crate) async fn accept_mailbox_delivery_for_current_turn(
|
||||
&self,
|
||||
active_turn: &Mutex<Option<ActiveTurn>>,
|
||||
sub_id: &str,
|
||||
) {
|
||||
let turn_state = self.turn_state_for_sub_id(active_turn, sub_id).await;
|
||||
let Some(turn_state) = turn_state else {
|
||||
return;
|
||||
};
|
||||
self.accept_mailbox_delivery_for_turn_state(turn_state.as_ref())
|
||||
.await;
|
||||
}
|
||||
|
||||
pub(super) async fn accept_mailbox_delivery_for_turn_state(
|
||||
&self,
|
||||
turn_state: &Mutex<TurnState>,
|
||||
) {
|
||||
turn_state
|
||||
.lock()
|
||||
.await
|
||||
.accept_mailbox_delivery_for_current_turn();
|
||||
}
|
||||
|
||||
pub(super) async fn push_pending_input_and_accept_mailbox_delivery_for_turn_state(
|
||||
&self,
|
||||
turn_state: &Mutex<TurnState>,
|
||||
input: ResponseInputItem,
|
||||
) {
|
||||
let mut turn_state = turn_state.lock().await;
|
||||
turn_state.pending_input.items.push(input);
|
||||
turn_state.accept_mailbox_delivery_for_current_turn();
|
||||
}
|
||||
|
||||
pub(crate) async fn extend_pending_input_for_turn_state(
|
||||
&self,
|
||||
turn_state: &Mutex<TurnState>,
|
||||
input: Vec<ResponseInputItem>,
|
||||
) {
|
||||
turn_state.lock().await.pending_input.items.extend(input);
|
||||
}
|
||||
|
||||
pub(crate) async fn take_pending_input_for_turn_state(
|
||||
&self,
|
||||
turn_state: &Mutex<TurnState>,
|
||||
) -> Vec<ResponseInputItem> {
|
||||
turn_state.lock().await.pending_input.items.split_off(0)
|
||||
}
|
||||
|
||||
#[expect(
|
||||
clippy::await_holding_invalid_type,
|
||||
reason = "active turn checks and turn state updates must remain atomic"
|
||||
)]
|
||||
pub(crate) async fn inject_response_items(
|
||||
&self,
|
||||
active_turn: &Mutex<Option<ActiveTurn>>,
|
||||
input: Vec<ResponseInputItem>,
|
||||
) -> Result<(), Vec<ResponseInputItem>> {
|
||||
let mut active = active_turn.lock().await;
|
||||
match active.as_mut() {
|
||||
Some(active_turn) => {
|
||||
active_turn
|
||||
.turn_state
|
||||
.lock()
|
||||
.await
|
||||
.pending_input
|
||||
.items
|
||||
.extend(input);
|
||||
Ok(())
|
||||
}
|
||||
None => Err(input),
|
||||
}
|
||||
}
|
||||
|
||||
#[expect(
|
||||
clippy::await_holding_invalid_type,
|
||||
reason = "active turn checks and turn state updates must remain atomic"
|
||||
)]
|
||||
pub(crate) async fn prepend_pending_input(
|
||||
&self,
|
||||
active_turn: &Mutex<Option<ActiveTurn>>,
|
||||
mut input: Vec<ResponseInputItem>,
|
||||
) -> Result<(), ()> {
|
||||
let mut active = active_turn.lock().await;
|
||||
match active.as_mut() {
|
||||
Some(active_turn) => {
|
||||
let mut turn_state = active_turn.turn_state.lock().await;
|
||||
if !input.is_empty() {
|
||||
let pending_input = &mut turn_state.pending_input;
|
||||
input.append(&mut pending_input.items);
|
||||
pending_input.items = input;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
None => Err(()),
|
||||
}
|
||||
}
|
||||
|
||||
#[expect(
|
||||
clippy::await_holding_invalid_type,
|
||||
reason = "active turn checks and turn state updates must remain atomic"
|
||||
)]
|
||||
pub(crate) async fn get_pending_input(
|
||||
&self,
|
||||
active_turn: &Mutex<Option<ActiveTurn>>,
|
||||
) -> Vec<ResponseInputItem> {
|
||||
let (pending_input, accepts_mailbox_delivery) = {
|
||||
let mut active = active_turn.lock().await;
|
||||
match active.as_mut() {
|
||||
Some(active_turn) => {
|
||||
let mut turn_state = active_turn.turn_state.lock().await;
|
||||
(
|
||||
turn_state.pending_input.items.split_off(0),
|
||||
turn_state.accepts_mailbox_delivery_for_current_turn(),
|
||||
)
|
||||
}
|
||||
None => (Vec::new(), true),
|
||||
}
|
||||
};
|
||||
if !accepts_mailbox_delivery {
|
||||
return pending_input;
|
||||
}
|
||||
let mailbox_items = self.drain_mailbox_input_items().await;
|
||||
if pending_input.is_empty() {
|
||||
mailbox_items
|
||||
} else if mailbox_items.is_empty() {
|
||||
pending_input
|
||||
} else {
|
||||
let mut pending_input = pending_input;
|
||||
pending_input.extend(mailbox_items);
|
||||
pending_input
|
||||
}
|
||||
}
|
||||
|
||||
#[expect(
|
||||
clippy::await_holding_invalid_type,
|
||||
reason = "active turn checks and turn state reads must remain atomic"
|
||||
)]
|
||||
pub(crate) async fn has_pending_input(&self, active_turn: &Mutex<Option<ActiveTurn>>) -> bool {
|
||||
let (has_turn_pending_input, accepts_mailbox_delivery) = {
|
||||
let active = active_turn.lock().await;
|
||||
match active.as_ref() {
|
||||
Some(active_turn) => {
|
||||
let turn_state = active_turn.turn_state.lock().await;
|
||||
(
|
||||
!turn_state.pending_input.items.is_empty(),
|
||||
turn_state.accepts_mailbox_delivery_for_current_turn(),
|
||||
)
|
||||
}
|
||||
None => (false, true),
|
||||
}
|
||||
};
|
||||
if has_turn_pending_input {
|
||||
return true;
|
||||
}
|
||||
if !accepts_mailbox_delivery {
|
||||
return false;
|
||||
}
|
||||
self.has_pending_mailbox_items().await
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use codex_protocol::AgentPath;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
fn make_mail(
|
||||
author: AgentPath,
|
||||
recipient: AgentPath,
|
||||
content: &str,
|
||||
trigger_turn: bool,
|
||||
) -> InterAgentCommunication {
|
||||
InterAgentCommunication::new(
|
||||
author,
|
||||
recipient,
|
||||
Vec::new(),
|
||||
content.to_string(),
|
||||
trigger_turn,
|
||||
)
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn input_queue_notifies_mailbox_subscribers() {
|
||||
let input_queue = InputQueue::new();
|
||||
let mut mailbox_rx = input_queue.subscribe_mailbox().await;
|
||||
|
||||
input_queue
|
||||
.enqueue_mailbox_communication(make_mail(
|
||||
AgentPath::root(),
|
||||
AgentPath::try_from("/root/worker").expect("agent path"),
|
||||
"one",
|
||||
/*trigger_turn*/ false,
|
||||
))
|
||||
.await;
|
||||
input_queue
|
||||
.enqueue_mailbox_communication(make_mail(
|
||||
AgentPath::root(),
|
||||
AgentPath::try_from("/root/worker").expect("agent path"),
|
||||
"two",
|
||||
/*trigger_turn*/ false,
|
||||
))
|
||||
.await;
|
||||
|
||||
mailbox_rx.changed().await.expect("mailbox update");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn input_queue_drains_mailbox_in_delivery_order() {
|
||||
let input_queue = InputQueue::new();
|
||||
let mail_one = make_mail(
|
||||
AgentPath::root(),
|
||||
AgentPath::try_from("/root/worker").expect("agent path"),
|
||||
"one",
|
||||
/*trigger_turn*/ false,
|
||||
);
|
||||
let mail_two = make_mail(
|
||||
AgentPath::try_from("/root/worker").expect("agent path"),
|
||||
AgentPath::root(),
|
||||
"two",
|
||||
/*trigger_turn*/ false,
|
||||
);
|
||||
|
||||
input_queue
|
||||
.enqueue_mailbox_communication(mail_one.clone())
|
||||
.await;
|
||||
input_queue
|
||||
.enqueue_mailbox_communication(mail_two.clone())
|
||||
.await;
|
||||
|
||||
assert_eq!(
|
||||
input_queue.drain_mailbox_input_items().await,
|
||||
vec![
|
||||
mail_one.to_response_input_item(),
|
||||
mail_two.to_response_input_item()
|
||||
]
|
||||
);
|
||||
assert!(!input_queue.has_pending_mailbox_items().await);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn input_queue_tracks_pending_trigger_turn_mail() {
|
||||
let input_queue = InputQueue::new();
|
||||
|
||||
input_queue
|
||||
.enqueue_mailbox_communication(make_mail(
|
||||
AgentPath::root(),
|
||||
AgentPath::try_from("/root/worker").expect("agent path"),
|
||||
"queued",
|
||||
/*trigger_turn*/ false,
|
||||
))
|
||||
.await;
|
||||
assert!(!input_queue.has_trigger_turn_mailbox_items().await);
|
||||
|
||||
input_queue
|
||||
.enqueue_mailbox_communication(make_mail(
|
||||
AgentPath::root(),
|
||||
AgentPath::try_from("/root/worker").expect("agent path"),
|
||||
"wake",
|
||||
/*trigger_turn*/ true,
|
||||
))
|
||||
.await;
|
||||
assert!(input_queue.has_trigger_turn_mailbox_items().await);
|
||||
}
|
||||
}
|
||||
@@ -10,8 +10,6 @@ use std::time::UNIX_EPOCH;
|
||||
|
||||
use crate::agent::AgentControl;
|
||||
use crate::agent::AgentStatus;
|
||||
use crate::agent::Mailbox;
|
||||
use crate::agent::MailboxReceiver;
|
||||
use crate::agent::agent_status_from_event;
|
||||
use crate::agent::status::is_final;
|
||||
use crate::attestation::AttestationProvider;
|
||||
@@ -193,6 +191,7 @@ use codex_protocol::exec_output::StreamOutput;
|
||||
|
||||
mod config_lock;
|
||||
mod handlers;
|
||||
mod input_queue;
|
||||
mod mcp;
|
||||
mod multi_agents;
|
||||
mod review;
|
||||
@@ -206,6 +205,7 @@ use self::config_lock::validate_config_lock_if_configured;
|
||||
#[cfg(test)]
|
||||
use self::handlers::submission_dispatch_span;
|
||||
use self::handlers::submission_loop;
|
||||
pub(crate) use self::input_queue::TurnInputQueue;
|
||||
use self::review::spawn_review_thread;
|
||||
use self::session::AppServerClientMetadata;
|
||||
use self::session::Session;
|
||||
@@ -289,8 +289,6 @@ use crate::rollout::map_session_init_error;
|
||||
use crate::session_startup_prewarm::SessionStartupPrewarmHandle;
|
||||
use crate::shell;
|
||||
use crate::shell_snapshot::ShellSnapshot;
|
||||
use crate::state::ActiveTurn;
|
||||
use crate::state::MailboxDeliveryPhase;
|
||||
use crate::state::PendingRequestPermissions;
|
||||
use crate::state::SessionServices;
|
||||
use crate::state::SessionState;
|
||||
@@ -2947,12 +2945,14 @@ impl Session {
|
||||
};
|
||||
if let Some(token_info) = token_info.as_ref() {
|
||||
for contributor in self.services.extensions.token_usage_contributors() {
|
||||
contributor.on_token_usage(
|
||||
&self.services.session_extension_data,
|
||||
&self.services.thread_extension_data,
|
||||
turn_context.extension_data.as_ref(),
|
||||
token_info,
|
||||
);
|
||||
contributor
|
||||
.on_token_usage(
|
||||
&self.services.session_extension_data,
|
||||
&self.services.thread_extension_data,
|
||||
turn_context.extension_data.as_ref(),
|
||||
token_info,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -3020,16 +3020,6 @@ impl Session {
|
||||
state.record_mcp_dependency_prompted(names);
|
||||
}
|
||||
|
||||
pub async fn dependency_env(&self) -> HashMap<String, String> {
|
||||
let state = self.state.lock().await;
|
||||
state.dependency_env()
|
||||
}
|
||||
|
||||
pub async fn set_dependency_env(&self, values: HashMap<String, String>) {
|
||||
let mut state = self.state.lock().await;
|
||||
state.set_dependency_env(values);
|
||||
}
|
||||
|
||||
pub(crate) async fn set_server_reasoning_included(&self, included: bool) {
|
||||
let mut state = self.state.lock().await;
|
||||
state.set_server_reasoning_included(included);
|
||||
@@ -3162,199 +3152,39 @@ impl Session {
|
||||
.set_responsesapi_client_metadata(responsesapi_client_metadata);
|
||||
}
|
||||
|
||||
let mut turn_state = active_turn.turn_state.lock().await;
|
||||
turn_state.push_pending_input(input.into());
|
||||
turn_state.accept_mailbox_delivery_for_current_turn();
|
||||
self.input_queue
|
||||
.push_pending_input_and_accept_mailbox_delivery_for_turn_state(
|
||||
active_turn.turn_state.as_ref(),
|
||||
input.into(),
|
||||
)
|
||||
.await;
|
||||
Ok(active_turn_id.clone())
|
||||
}
|
||||
|
||||
/// Returns the input if there was no task running to inject into.
|
||||
#[expect(
|
||||
clippy::await_holding_invalid_type,
|
||||
reason = "active turn checks and turn state updates must remain atomic"
|
||||
)]
|
||||
pub async fn inject_response_items(
|
||||
&self,
|
||||
input: Vec<ResponseInputItem>,
|
||||
) -> Result<(), Vec<ResponseInputItem>> {
|
||||
let mut active = self.active_turn.lock().await;
|
||||
match active.as_mut() {
|
||||
Some(at) => {
|
||||
let mut ts = at.turn_state.lock().await;
|
||||
for item in input {
|
||||
ts.push_pending_input(item);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
None => Err(input),
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) async fn defer_mailbox_delivery_to_next_turn(&self, sub_id: &str) {
|
||||
let turn_state = self.turn_state_for_sub_id(sub_id).await;
|
||||
let Some(turn_state) = turn_state else {
|
||||
return;
|
||||
};
|
||||
let mut turn_state = turn_state.lock().await;
|
||||
if turn_state.has_pending_input() {
|
||||
return;
|
||||
}
|
||||
turn_state.set_mailbox_delivery_phase(MailboxDeliveryPhase::NextTurn);
|
||||
}
|
||||
|
||||
pub(crate) async fn accept_mailbox_delivery_for_current_turn(&self, sub_id: &str) {
|
||||
let turn_state = self.turn_state_for_sub_id(sub_id).await;
|
||||
let Some(turn_state) = turn_state else {
|
||||
return;
|
||||
};
|
||||
turn_state
|
||||
.lock()
|
||||
self.input_queue
|
||||
.inject_response_items(&self.active_turn, input)
|
||||
.await
|
||||
.set_mailbox_delivery_phase(MailboxDeliveryPhase::CurrentTurn);
|
||||
}
|
||||
|
||||
pub(crate) async fn record_memory_citation_for_turn(&self, sub_id: &str) {
|
||||
let turn_state = self.turn_state_for_sub_id(sub_id).await;
|
||||
let turn_state = self
|
||||
.input_queue
|
||||
.turn_state_for_sub_id(&self.active_turn, sub_id)
|
||||
.await;
|
||||
let Some(turn_state) = turn_state else {
|
||||
return;
|
||||
};
|
||||
turn_state.lock().await.has_memory_citation = true;
|
||||
}
|
||||
|
||||
async fn turn_state_for_sub_id(
|
||||
&self,
|
||||
sub_id: &str,
|
||||
) -> Option<Arc<tokio::sync::Mutex<crate::state::TurnState>>> {
|
||||
let active = self.active_turn.lock().await;
|
||||
active.as_ref().and_then(|active_turn| {
|
||||
active_turn
|
||||
.tasks
|
||||
.contains_key(sub_id)
|
||||
.then(|| Arc::clone(&active_turn.turn_state))
|
||||
})
|
||||
}
|
||||
|
||||
pub(crate) fn subscribe_mailbox_seq(&self) -> watch::Receiver<u64> {
|
||||
self.mailbox.subscribe()
|
||||
}
|
||||
|
||||
pub(crate) fn enqueue_mailbox_communication(&self, communication: InterAgentCommunication) {
|
||||
self.mailbox.send(communication);
|
||||
}
|
||||
|
||||
pub(crate) async fn has_trigger_turn_mailbox_items(&self) -> bool {
|
||||
self.mailbox_rx.lock().await.has_pending_trigger_turn()
|
||||
}
|
||||
|
||||
pub(crate) async fn has_pending_mailbox_items(&self) -> bool {
|
||||
self.mailbox_rx.lock().await.has_pending()
|
||||
}
|
||||
|
||||
#[expect(
|
||||
clippy::await_holding_invalid_type,
|
||||
reason = "active turn checks and turn state updates must remain atomic"
|
||||
)]
|
||||
pub async fn prepend_pending_input(&self, input: Vec<ResponseInputItem>) -> Result<(), ()> {
|
||||
let mut active = self.active_turn.lock().await;
|
||||
match active.as_mut() {
|
||||
Some(at) => {
|
||||
let mut ts = at.turn_state.lock().await;
|
||||
ts.prepend_pending_input(input);
|
||||
Ok(())
|
||||
}
|
||||
None => Err(()),
|
||||
}
|
||||
}
|
||||
|
||||
#[expect(
|
||||
clippy::await_holding_invalid_type,
|
||||
reason = "active turn checks and turn state updates must remain atomic"
|
||||
)]
|
||||
pub async fn get_pending_input(&self) -> Vec<ResponseInputItem> {
|
||||
let (pending_input, accepts_mailbox_delivery) = {
|
||||
let mut active = self.active_turn.lock().await;
|
||||
match active.as_mut() {
|
||||
Some(at) => {
|
||||
let mut ts = at.turn_state.lock().await;
|
||||
(
|
||||
ts.take_pending_input(),
|
||||
ts.accepts_mailbox_delivery_for_current_turn(),
|
||||
)
|
||||
}
|
||||
None => (Vec::new(), true),
|
||||
}
|
||||
};
|
||||
if !accepts_mailbox_delivery {
|
||||
return pending_input;
|
||||
}
|
||||
let mailbox_items = {
|
||||
let mut mailbox_rx = self.mailbox_rx.lock().await;
|
||||
mailbox_rx
|
||||
.drain()
|
||||
.into_iter()
|
||||
.map(|mail| mail.to_response_input_item())
|
||||
.collect::<Vec<_>>()
|
||||
};
|
||||
if pending_input.is_empty() {
|
||||
mailbox_items
|
||||
} else if mailbox_items.is_empty() {
|
||||
pending_input
|
||||
} else {
|
||||
let mut pending_input = pending_input;
|
||||
pending_input.extend(mailbox_items);
|
||||
pending_input
|
||||
}
|
||||
}
|
||||
|
||||
/// Queue response items to be injected into the next active turn created for this session.
|
||||
pub(crate) async fn queue_response_items_for_next_turn(&self, items: Vec<ResponseInputItem>) {
|
||||
if items.is_empty() {
|
||||
return;
|
||||
}
|
||||
|
||||
let mut idle_pending_input = self.idle_pending_input.lock().await;
|
||||
idle_pending_input.extend(items);
|
||||
}
|
||||
|
||||
pub(crate) async fn take_queued_response_items_for_next_turn(&self) -> Vec<ResponseInputItem> {
|
||||
std::mem::take(&mut *self.idle_pending_input.lock().await)
|
||||
}
|
||||
|
||||
pub(crate) async fn has_queued_response_items_for_next_turn(&self) -> bool {
|
||||
!self.idle_pending_input.lock().await.is_empty()
|
||||
}
|
||||
|
||||
#[expect(
|
||||
clippy::await_holding_invalid_type,
|
||||
reason = "active turn checks and turn state reads must remain atomic"
|
||||
)]
|
||||
pub async fn has_pending_input(&self) -> bool {
|
||||
let (has_turn_pending_input, accepts_mailbox_delivery) = {
|
||||
let active = self.active_turn.lock().await;
|
||||
match active.as_ref() {
|
||||
Some(at) => {
|
||||
let ts = at.turn_state.lock().await;
|
||||
(
|
||||
ts.has_pending_input(),
|
||||
ts.accepts_mailbox_delivery_for_current_turn(),
|
||||
)
|
||||
}
|
||||
None => (false, true),
|
||||
}
|
||||
};
|
||||
if has_turn_pending_input {
|
||||
return true;
|
||||
}
|
||||
if !accepts_mailbox_delivery {
|
||||
return false;
|
||||
}
|
||||
self.has_pending_mailbox_items().await
|
||||
}
|
||||
|
||||
pub async fn interrupt_task(self: &Arc<Self>) {
|
||||
info!("interrupt received: abort current task, if any");
|
||||
let had_active_turn = self.active_turn.lock().await.is_some();
|
||||
// Even without an active task, interrupt handling pauses any active goal.
|
||||
self.abort_all_tasks(TurnAbortReason::Interrupted).await;
|
||||
if !had_active_turn {
|
||||
self.cancel_mcp_startup().await;
|
||||
|
||||
@@ -56,6 +56,7 @@ pub(super) async fn spawn_review_thread(
|
||||
.with_spawn_agent_usage_hint(config.multi_agent_v2.usage_hint_enabled)
|
||||
.with_spawn_agent_usage_hint_text(config.multi_agent_v2.usage_hint_text.clone())
|
||||
.with_hide_spawn_agent_metadata(config.multi_agent_v2.hide_spawn_agent_metadata)
|
||||
.with_multi_agent_v2_tool_namespace(config.multi_agent_v2.tool_namespace.clone())
|
||||
.with_multi_agent_v2_non_code_mode_only(config.multi_agent_v2.non_code_mode_only)
|
||||
.with_goal_tools_allowed(goal_tools_supported)
|
||||
.with_max_concurrent_threads_per_session(config.agent_max_threads)
|
||||
|
||||
@@ -59,7 +59,6 @@ async fn record_initial_history_resumed_bare_turn_context_does_not_hydrate_previ
|
||||
let previous_model = "previous-rollout-model";
|
||||
let previous_context_item = TurnContextItem {
|
||||
turn_id: Some(turn_context.sub_id.clone()),
|
||||
trace_id: turn_context.trace_id.clone(),
|
||||
#[allow(deprecated)]
|
||||
cwd: turn_context.cwd.to_path_buf(),
|
||||
current_date: turn_context.current_date.clone(),
|
||||
@@ -74,11 +73,7 @@ async fn record_initial_history_resumed_bare_turn_context_does_not_hydrate_previ
|
||||
collaboration_mode: Some(turn_context.collaboration_mode.clone()),
|
||||
realtime_active: Some(turn_context.realtime_active),
|
||||
effort: turn_context.reasoning_effort,
|
||||
summary: turn_context.reasoning_summary,
|
||||
user_instructions: None,
|
||||
developer_instructions: None,
|
||||
final_output_json_schema: None,
|
||||
truncation_policy: Some(turn_context.truncation_policy),
|
||||
summary: codex_protocol::config_types::ReasoningSummary::Auto,
|
||||
};
|
||||
let rollout_items = vec![RolloutItem::TurnContext(previous_context_item)];
|
||||
|
||||
@@ -101,7 +96,6 @@ async fn record_initial_history_resumed_hydrates_previous_turn_settings_from_lif
|
||||
let previous_model = "previous-rollout-model";
|
||||
let mut previous_context_item = TurnContextItem {
|
||||
turn_id: Some(turn_context.sub_id.clone()),
|
||||
trace_id: turn_context.trace_id.clone(),
|
||||
#[allow(deprecated)]
|
||||
cwd: turn_context.cwd.to_path_buf(),
|
||||
current_date: turn_context.current_date.clone(),
|
||||
@@ -116,11 +110,7 @@ async fn record_initial_history_resumed_hydrates_previous_turn_settings_from_lif
|
||||
collaboration_mode: Some(turn_context.collaboration_mode.clone()),
|
||||
realtime_active: Some(turn_context.realtime_active),
|
||||
effort: turn_context.reasoning_effort,
|
||||
summary: turn_context.reasoning_summary,
|
||||
user_instructions: None,
|
||||
developer_instructions: None,
|
||||
final_output_json_schema: None,
|
||||
truncation_policy: Some(turn_context.truncation_policy),
|
||||
summary: codex_protocol::config_types::ReasoningSummary::Auto,
|
||||
};
|
||||
let turn_id = previous_context_item
|
||||
.turn_id
|
||||
@@ -925,7 +915,6 @@ async fn record_initial_history_resumed_turn_context_after_compaction_reestablis
|
||||
let previous_model = "previous-rollout-model";
|
||||
let previous_context_item = TurnContextItem {
|
||||
turn_id: Some(turn_context.sub_id.clone()),
|
||||
trace_id: turn_context.trace_id.clone(),
|
||||
#[allow(deprecated)]
|
||||
cwd: turn_context.cwd.to_path_buf(),
|
||||
current_date: turn_context.current_date.clone(),
|
||||
@@ -940,11 +929,7 @@ async fn record_initial_history_resumed_turn_context_after_compaction_reestablis
|
||||
collaboration_mode: Some(turn_context.collaboration_mode.clone()),
|
||||
realtime_active: Some(turn_context.realtime_active),
|
||||
effort: turn_context.reasoning_effort,
|
||||
summary: turn_context.reasoning_summary,
|
||||
user_instructions: None,
|
||||
developer_instructions: None,
|
||||
final_output_json_schema: None,
|
||||
truncation_policy: Some(turn_context.truncation_policy),
|
||||
summary: codex_protocol::config_types::ReasoningSummary::Auto,
|
||||
};
|
||||
let previous_turn_id = previous_context_item
|
||||
.turn_id
|
||||
@@ -1005,7 +990,6 @@ async fn record_initial_history_resumed_turn_context_after_compaction_reestablis
|
||||
.expect("serialize seeded reference context item"),
|
||||
serde_json::to_value(Some(TurnContextItem {
|
||||
turn_id: Some(turn_context.sub_id.clone()),
|
||||
trace_id: turn_context.trace_id.clone(),
|
||||
#[allow(deprecated)]
|
||||
cwd: turn_context.cwd.to_path_buf(),
|
||||
current_date: turn_context.current_date.clone(),
|
||||
@@ -1020,11 +1004,7 @@ async fn record_initial_history_resumed_turn_context_after_compaction_reestablis
|
||||
collaboration_mode: Some(turn_context.collaboration_mode.clone()),
|
||||
realtime_active: Some(turn_context.realtime_active),
|
||||
effort: turn_context.reasoning_effort,
|
||||
summary: turn_context.reasoning_summary,
|
||||
user_instructions: None,
|
||||
developer_instructions: None,
|
||||
final_output_json_schema: None,
|
||||
truncation_policy: Some(turn_context.truncation_policy),
|
||||
summary: codex_protocol::config_types::ReasoningSummary::Auto,
|
||||
}))
|
||||
.expect("serialize expected reference context item")
|
||||
);
|
||||
@@ -1037,7 +1017,6 @@ async fn record_initial_history_resumed_aborted_turn_without_id_clears_active_tu
|
||||
let previous_model = "previous-rollout-model";
|
||||
let previous_context_item = TurnContextItem {
|
||||
turn_id: Some(turn_context.sub_id.clone()),
|
||||
trace_id: turn_context.trace_id.clone(),
|
||||
#[allow(deprecated)]
|
||||
cwd: turn_context.cwd.to_path_buf(),
|
||||
current_date: turn_context.current_date.clone(),
|
||||
@@ -1052,11 +1031,7 @@ async fn record_initial_history_resumed_aborted_turn_without_id_clears_active_tu
|
||||
collaboration_mode: Some(turn_context.collaboration_mode.clone()),
|
||||
realtime_active: Some(turn_context.realtime_active),
|
||||
effort: turn_context.reasoning_effort,
|
||||
summary: turn_context.reasoning_summary,
|
||||
user_instructions: None,
|
||||
developer_instructions: None,
|
||||
final_output_json_schema: None,
|
||||
truncation_policy: Some(turn_context.truncation_policy),
|
||||
summary: codex_protocol::config_types::ReasoningSummary::Auto,
|
||||
};
|
||||
let previous_turn_id = previous_context_item
|
||||
.turn_id
|
||||
@@ -1155,7 +1130,6 @@ async fn record_initial_history_resumed_unmatched_abort_preserves_active_turn_fo
|
||||
let unmatched_abort_turn_id = "other-turn".to_string();
|
||||
let current_context_item = TurnContextItem {
|
||||
turn_id: Some(current_turn_id.clone()),
|
||||
trace_id: turn_context.trace_id.clone(),
|
||||
#[allow(deprecated)]
|
||||
cwd: turn_context.cwd.to_path_buf(),
|
||||
current_date: turn_context.current_date.clone(),
|
||||
@@ -1170,11 +1144,7 @@ async fn record_initial_history_resumed_unmatched_abort_preserves_active_turn_fo
|
||||
collaboration_mode: Some(turn_context.collaboration_mode.clone()),
|
||||
realtime_active: Some(turn_context.realtime_active),
|
||||
effort: turn_context.reasoning_effort,
|
||||
summary: turn_context.reasoning_summary,
|
||||
user_instructions: None,
|
||||
developer_instructions: None,
|
||||
final_output_json_schema: None,
|
||||
truncation_policy: Some(turn_context.truncation_policy),
|
||||
summary: codex_protocol::config_types::ReasoningSummary::Auto,
|
||||
};
|
||||
|
||||
let rollout_items = vec![
|
||||
@@ -1272,7 +1242,6 @@ async fn record_initial_history_resumed_trailing_incomplete_turn_compaction_clea
|
||||
let previous_model = "previous-rollout-model";
|
||||
let previous_context_item = TurnContextItem {
|
||||
turn_id: Some(turn_context.sub_id.clone()),
|
||||
trace_id: turn_context.trace_id.clone(),
|
||||
#[allow(deprecated)]
|
||||
cwd: turn_context.cwd.to_path_buf(),
|
||||
current_date: turn_context.current_date.clone(),
|
||||
@@ -1287,11 +1256,7 @@ async fn record_initial_history_resumed_trailing_incomplete_turn_compaction_clea
|
||||
collaboration_mode: Some(turn_context.collaboration_mode.clone()),
|
||||
realtime_active: Some(turn_context.realtime_active),
|
||||
effort: turn_context.reasoning_effort,
|
||||
summary: turn_context.reasoning_summary,
|
||||
user_instructions: None,
|
||||
developer_instructions: None,
|
||||
final_output_json_schema: None,
|
||||
truncation_policy: Some(turn_context.truncation_policy),
|
||||
summary: codex_protocol::config_types::ReasoningSummary::Auto,
|
||||
};
|
||||
let previous_turn_id = previous_context_item
|
||||
.turn_id
|
||||
@@ -1428,7 +1393,6 @@ async fn record_initial_history_resumed_replaced_incomplete_compacted_turn_clear
|
||||
let previous_model = "previous-rollout-model";
|
||||
let previous_context_item = TurnContextItem {
|
||||
turn_id: Some(turn_context.sub_id.clone()),
|
||||
trace_id: turn_context.trace_id.clone(),
|
||||
#[allow(deprecated)]
|
||||
cwd: turn_context.cwd.to_path_buf(),
|
||||
current_date: turn_context.current_date.clone(),
|
||||
@@ -1443,11 +1407,7 @@ async fn record_initial_history_resumed_replaced_incomplete_compacted_turn_clear
|
||||
collaboration_mode: Some(turn_context.collaboration_mode.clone()),
|
||||
realtime_active: Some(turn_context.realtime_active),
|
||||
effort: turn_context.reasoning_effort,
|
||||
summary: turn_context.reasoning_summary,
|
||||
user_instructions: None,
|
||||
developer_instructions: None,
|
||||
final_output_json_schema: None,
|
||||
truncation_policy: Some(turn_context.truncation_policy),
|
||||
summary: codex_protocol::config_types::ReasoningSummary::Auto,
|
||||
};
|
||||
let previous_turn_id = previous_context_item
|
||||
.turn_id
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
use super::input_queue::InputQueue;
|
||||
use super::*;
|
||||
use crate::goals::GoalRuntimeState;
|
||||
use crate::state::ActiveTurn;
|
||||
use codex_protocol::SessionId;
|
||||
use codex_protocol::config_types::ServiceTier;
|
||||
use codex_protocol::permissions::FileSystemPath;
|
||||
@@ -27,9 +29,7 @@ pub(crate) struct Session {
|
||||
pub(super) pending_mcp_server_refresh_config: Mutex<Option<McpServerRefreshConfig>>,
|
||||
pub(crate) conversation: Arc<RealtimeConversationManager>,
|
||||
pub(crate) active_turn: Mutex<Option<ActiveTurn>>,
|
||||
pub(super) mailbox: Mailbox,
|
||||
pub(super) mailbox_rx: Mutex<MailboxReceiver>,
|
||||
pub(super) idle_pending_input: Mutex<Vec<ResponseInputItem>>, // TODO (jif) merge with mailbox!
|
||||
pub(crate) input_queue: InputQueue,
|
||||
pub(crate) goal_runtime: GoalRuntimeState,
|
||||
pub(crate) guardian_review_session: GuardianReviewSessionManager,
|
||||
pub(crate) services: SessionServices,
|
||||
@@ -877,7 +877,7 @@ impl Session {
|
||||
config: config.as_ref(),
|
||||
session_store: &session_extension_data,
|
||||
thread_store: &thread_extension_data,
|
||||
});
|
||||
}).await;
|
||||
}
|
||||
|
||||
let services = SessionServices {
|
||||
@@ -950,7 +950,6 @@ impl Session {
|
||||
let (out_of_band_elicitation_paused, _out_of_band_elicitation_paused_rx) =
|
||||
watch::channel(false);
|
||||
|
||||
let (mailbox, mailbox_rx) = Mailbox::new();
|
||||
let sess = Arc::new(Session {
|
||||
conversation_id: thread_id,
|
||||
installation_id,
|
||||
@@ -963,9 +962,7 @@ impl Session {
|
||||
pending_mcp_server_refresh_config: Mutex::new(None),
|
||||
conversation: Arc::new(RealtimeConversationManager::new()),
|
||||
active_turn: Mutex::new(None),
|
||||
mailbox,
|
||||
mailbox_rx: Mutex::new(mailbox_rx),
|
||||
idle_pending_input: Mutex::new(Vec::new()),
|
||||
input_queue: InputQueue::new(),
|
||||
goal_runtime: GoalRuntimeState::new(),
|
||||
guardian_review_session: GuardianReviewSessionManager::default(),
|
||||
services,
|
||||
@@ -1130,7 +1127,7 @@ impl Session {
|
||||
};
|
||||
|
||||
// record_initial_history can emit events. We record only after the SessionConfiguredEvent is emitted.
|
||||
sess.record_initial_history(initial_history).await;
|
||||
Box::pin(sess.record_initial_history(initial_history)).await;
|
||||
{
|
||||
let mut state = sess.state.lock().await;
|
||||
state.set_pending_session_start_source(Some(session_start_source));
|
||||
|
||||
@@ -1874,8 +1874,9 @@ async fn record_token_usage_info_notifies_extension_contributors() {
|
||||
records: Arc<std::sync::Mutex<Vec<RecordedTokenUsage>>>,
|
||||
}
|
||||
|
||||
#[async_trait::async_trait]
|
||||
impl codex_extension_api::TokenUsageContributor for TokenUsageRecorder {
|
||||
fn on_token_usage(
|
||||
async fn on_token_usage(
|
||||
&self,
|
||||
session_store: &codex_extension_api::ExtensionData,
|
||||
thread_store: &codex_extension_api::ExtensionData,
|
||||
@@ -2318,7 +2319,6 @@ async fn record_initial_history_forked_hydrates_previous_turn_settings() {
|
||||
let previous_model = "forked-rollout-model";
|
||||
let previous_context_item = TurnContextItem {
|
||||
turn_id: Some(turn_context.sub_id.clone()),
|
||||
trace_id: turn_context.trace_id.clone(),
|
||||
#[allow(deprecated)]
|
||||
cwd: turn_context.cwd.to_path_buf(),
|
||||
current_date: turn_context.current_date.clone(),
|
||||
@@ -2333,11 +2333,7 @@ async fn record_initial_history_forked_hydrates_previous_turn_settings() {
|
||||
collaboration_mode: Some(turn_context.collaboration_mode.clone()),
|
||||
realtime_active: Some(turn_context.realtime_active),
|
||||
effort: turn_context.reasoning_effort,
|
||||
summary: turn_context.reasoning_summary,
|
||||
user_instructions: None,
|
||||
developer_instructions: None,
|
||||
final_output_json_schema: None,
|
||||
truncation_policy: Some(turn_context.truncation_policy),
|
||||
summary: codex_protocol::config_types::ReasoningSummary::Auto,
|
||||
};
|
||||
let turn_id = previous_context_item
|
||||
.turn_id
|
||||
@@ -4400,7 +4396,6 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
|
||||
/*goal_tools_supported*/ true,
|
||||
);
|
||||
|
||||
let (mailbox, mailbox_rx) = crate::agent::Mailbox::new();
|
||||
let session = Session {
|
||||
conversation_id: thread_id,
|
||||
installation_id: "11111111-1111-4111-8111-111111111111".to_string(),
|
||||
@@ -4413,9 +4408,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
|
||||
pending_mcp_server_refresh_config: Mutex::new(None),
|
||||
conversation: Arc::new(RealtimeConversationManager::new()),
|
||||
active_turn: Mutex::new(None),
|
||||
mailbox,
|
||||
mailbox_rx: Mutex::new(mailbox_rx),
|
||||
idle_pending_input: Mutex::new(Vec::new()),
|
||||
input_queue: super::input_queue::InputQueue::new(),
|
||||
goal_runtime: crate::goals::GoalRuntimeState::new(),
|
||||
guardian_review_session: crate::guardian::GuardianReviewSessionManager::default(),
|
||||
services,
|
||||
@@ -5126,7 +5119,7 @@ async fn new_default_turn_captures_current_span_trace_id() {
|
||||
&request_parent
|
||||
));
|
||||
|
||||
let turn_context_item = async {
|
||||
let turn_trace_id = async {
|
||||
let expected_trace_id = Span::current()
|
||||
.context()
|
||||
.span()
|
||||
@@ -5134,15 +5127,14 @@ async fn new_default_turn_captures_current_span_trace_id() {
|
||||
.trace_id()
|
||||
.to_string();
|
||||
let turn_context = session.new_default_turn().await;
|
||||
let turn_context_item = turn_context.to_turn_context_item();
|
||||
assert_eq!(turn_context_item.trace_id, Some(expected_trace_id));
|
||||
turn_context_item
|
||||
assert_eq!(turn_context.trace_id, Some(expected_trace_id));
|
||||
turn_context.trace_id.clone()
|
||||
}
|
||||
.instrument(request_span)
|
||||
.await;
|
||||
|
||||
assert_eq!(
|
||||
turn_context_item.trace_id.as_deref(),
|
||||
turn_trace_id.as_deref(),
|
||||
Some("00000000000000000000000000000011")
|
||||
);
|
||||
}
|
||||
@@ -5668,8 +5660,9 @@ async fn submission_loop_channel_close_emits_thread_stop_lifecycle() {
|
||||
expected_thread_id: ThreadId,
|
||||
}
|
||||
|
||||
#[async_trait::async_trait]
|
||||
impl codex_extension_api::ThreadLifecycleContributor<crate::config::Config> for ThreadStopRecorder {
|
||||
fn on_thread_stop(&self, input: codex_extension_api::ThreadStopInput<'_>) {
|
||||
async fn on_thread_stop(&self, input: codex_extension_api::ThreadStopInput<'_>) {
|
||||
assert_eq!(
|
||||
self.expected_thread_id.to_string(),
|
||||
input.thread_store.level_id()
|
||||
@@ -5713,8 +5706,9 @@ async fn submission_loop_channel_close_aborts_active_turn_before_thread_stop_lif
|
||||
expected_turn_id: String,
|
||||
}
|
||||
|
||||
#[async_trait::async_trait]
|
||||
impl codex_extension_api::ThreadLifecycleContributor<crate::config::Config> for LifecycleRecorder {
|
||||
fn on_thread_stop(&self, input: codex_extension_api::ThreadStopInput<'_>) {
|
||||
async fn on_thread_stop(&self, input: codex_extension_api::ThreadStopInput<'_>) {
|
||||
assert_eq!(
|
||||
self.expected_thread_id.to_string(),
|
||||
input.thread_store.level_id()
|
||||
@@ -5726,8 +5720,9 @@ async fn submission_loop_channel_close_aborts_active_turn_before_thread_stop_lif
|
||||
}
|
||||
}
|
||||
|
||||
#[async_trait::async_trait]
|
||||
impl codex_extension_api::TurnLifecycleContributor for LifecycleRecorder {
|
||||
fn on_turn_abort(&self, input: codex_extension_api::TurnAbortInput<'_>) {
|
||||
async fn on_turn_abort(&self, input: codex_extension_api::TurnAbortInput<'_>) {
|
||||
assert_eq!(
|
||||
self.expected_thread_id.to_string(),
|
||||
input.thread_store.level_id()
|
||||
@@ -6239,7 +6234,6 @@ where
|
||||
/*goal_tools_supported*/ true,
|
||||
));
|
||||
|
||||
let (mailbox, mailbox_rx) = crate::agent::Mailbox::new();
|
||||
let session = Arc::new(Session {
|
||||
conversation_id: thread_id,
|
||||
installation_id: "11111111-1111-4111-8111-111111111111".to_string(),
|
||||
@@ -6252,9 +6246,7 @@ where
|
||||
pending_mcp_server_refresh_config: Mutex::new(None),
|
||||
conversation: Arc::new(RealtimeConversationManager::new()),
|
||||
active_turn: Mutex::new(None),
|
||||
mailbox,
|
||||
mailbox_rx: Mutex::new(mailbox_rx),
|
||||
idle_pending_input: Mutex::new(Vec::new()),
|
||||
input_queue: super::input_queue::InputQueue::new(),
|
||||
goal_runtime: crate::goals::GoalRuntimeState::new(),
|
||||
guardian_review_session: crate::guardian::GuardianReviewSessionManager::default(),
|
||||
services,
|
||||
@@ -8085,7 +8077,7 @@ async fn steer_input_returns_active_turn_id() {
|
||||
.expect("steering with matching expected turn id should succeed");
|
||||
|
||||
assert_eq!(turn_id, tc.sub_id);
|
||||
assert!(sess.has_pending_input().await);
|
||||
assert!(sess.input_queue.has_pending_input(&sess.active_turn).await);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -8131,7 +8123,7 @@ async fn prepend_pending_input_keeps_older_tail_ahead_of_newer_input() {
|
||||
.await
|
||||
.expect("inject initial pending input into active turn");
|
||||
|
||||
let drained = sess.get_pending_input().await;
|
||||
let drained = sess.input_queue.get_pending_input(&sess.active_turn).await;
|
||||
assert_eq!(drained, vec![blocked, later.clone()]);
|
||||
|
||||
sess.inject_response_items(vec![newer.clone()])
|
||||
@@ -8140,11 +8132,15 @@ async fn prepend_pending_input_keeps_older_tail_ahead_of_newer_input() {
|
||||
|
||||
let mut drained_iter = drained.into_iter();
|
||||
let _blocked = drained_iter.next().expect("blocked prompt should exist");
|
||||
sess.prepend_pending_input(drained_iter.collect())
|
||||
sess.input_queue
|
||||
.prepend_pending_input(&sess.active_turn, drained_iter.collect())
|
||||
.await
|
||||
.expect("requeue later pending input at the front of the queue");
|
||||
|
||||
assert_eq!(sess.get_pending_input().await, vec![later, newer]);
|
||||
assert_eq!(
|
||||
sess.input_queue.get_pending_input(&sess.active_turn).await,
|
||||
vec![later, newer]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -8158,7 +8154,8 @@ async fn queued_response_items_for_next_turn_move_into_next_active_turn() {
|
||||
phase: None,
|
||||
};
|
||||
|
||||
sess.queue_response_items_for_next_turn(vec![queued_item.clone()])
|
||||
sess.input_queue
|
||||
.queue_response_items_for_next_turn(vec![queued_item.clone()])
|
||||
.await;
|
||||
|
||||
sess.spawn_task(
|
||||
@@ -8171,7 +8168,10 @@ async fn queued_response_items_for_next_turn_move_into_next_active_turn() {
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(sess.get_pending_input().await, vec![queued_item]);
|
||||
assert_eq!(
|
||||
sess.input_queue.get_pending_input(&sess.active_turn).await,
|
||||
vec![queued_item]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -8185,13 +8185,18 @@ async fn idle_interrupt_does_not_wake_queued_next_turn_items() {
|
||||
phase: None,
|
||||
};
|
||||
|
||||
sess.queue_response_items_for_next_turn(vec![queued_item])
|
||||
sess.input_queue
|
||||
.queue_response_items_for_next_turn(vec![queued_item])
|
||||
.await;
|
||||
|
||||
sess.abort_all_tasks(TurnAbortReason::Interrupted).await;
|
||||
|
||||
assert!(sess.active_turn.lock().await.is_none());
|
||||
assert!(sess.has_queued_response_items_for_next_turn().await);
|
||||
assert!(
|
||||
sess.input_queue
|
||||
.has_queued_response_items_for_next_turn()
|
||||
.await
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -8209,22 +8214,23 @@ async fn abort_empty_active_turn_preserves_pending_input() {
|
||||
let active_turn = active.get_or_insert_with(ActiveTurn::default);
|
||||
Arc::clone(&active_turn.turn_state)
|
||||
};
|
||||
turn_state
|
||||
.lock()
|
||||
.await
|
||||
.push_pending_input(pending_item.clone());
|
||||
sess.input_queue
|
||||
.extend_pending_input_for_turn_state(turn_state.as_ref(), vec![pending_item.clone()])
|
||||
.await;
|
||||
|
||||
sess.abort_all_tasks(TurnAbortReason::Replaced).await;
|
||||
|
||||
assert!(sess.active_turn.lock().await.is_none());
|
||||
assert_eq!(
|
||||
turn_state.lock().await.take_pending_input(),
|
||||
sess.input_queue
|
||||
.take_pending_input_for_turn_state(turn_state.as_ref())
|
||||
.await,
|
||||
vec![pending_item]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn interrupt_accounts_active_goal_before_pausing() -> anyhow::Result<()> {
|
||||
async fn interrupt_accounts_active_goal_without_pausing() -> anyhow::Result<()> {
|
||||
let (sess, tc, _rx, _codex_home) = make_goal_session_and_context_with_rx().await;
|
||||
sess.set_thread_goal(
|
||||
tc.as_ref(),
|
||||
@@ -8254,7 +8260,7 @@ async fn interrupt_accounts_active_goal_before_pausing() -> anyhow::Result<()> {
|
||||
.await?
|
||||
.expect("goal should remain persisted after interrupt");
|
||||
assert_eq!(
|
||||
codex_protocol::protocol::ThreadGoalStatus::Paused,
|
||||
codex_protocol::protocol::ThreadGoalStatus::Active,
|
||||
goal.status
|
||||
);
|
||||
assert_eq!(70, goal.tokens_used);
|
||||
@@ -8264,6 +8270,34 @@ async fn interrupt_accounts_active_goal_before_pausing() -> anyhow::Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn shutdown_without_active_turn_keeps_active_goal_active() -> anyhow::Result<()> {
|
||||
let (sess, tc, _rx, _codex_home) = make_goal_session_and_context_with_rx().await;
|
||||
sess.set_thread_goal(
|
||||
tc.as_ref(),
|
||||
SetGoalRequest {
|
||||
objective: Some("Keep improving the benchmark".to_string()),
|
||||
status: None,
|
||||
token_budget: None,
|
||||
},
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert!(sess.active_turn.lock().await.is_none());
|
||||
assert!(handlers::shutdown(&sess, "shutdown".to_string()).await);
|
||||
|
||||
let goal = sess
|
||||
.get_thread_goal()
|
||||
.await?
|
||||
.expect("goal should remain persisted after shutdown");
|
||||
assert_eq!(
|
||||
codex_protocol::protocol::ThreadGoalStatus::Active,
|
||||
goal.status
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn active_goal_continuation_runs_again_after_no_tool_turn() -> anyhow::Result<()> {
|
||||
let server = start_mock_server().await;
|
||||
@@ -8554,7 +8588,7 @@ async fn budget_limited_accounting_steers_active_turn_without_aborting() -> anyh
|
||||
})
|
||||
.await?;
|
||||
|
||||
let pending_input = sess.get_pending_input().await;
|
||||
let pending_input = sess.input_queue.get_pending_input(&sess.active_turn).await;
|
||||
let [ResponseInputItem::Message { role, content, .. }] = pending_input.as_slice() else {
|
||||
panic!("expected one budget-limit steering message, got {pending_input:#?}");
|
||||
};
|
||||
@@ -8576,6 +8610,7 @@ async fn budget_limited_accounting_steers_active_turn_without_aborting() -> anyh
|
||||
|
||||
let state_db = goal_test_state_db(sess.as_ref()).await?;
|
||||
let goal = state_db
|
||||
.thread_goals()
|
||||
.get_thread_goal(sess.conversation_id)
|
||||
.await?
|
||||
.expect("goal should remain persisted after accounting");
|
||||
@@ -8599,6 +8634,7 @@ async fn budget_limited_accounting_steers_active_turn_without_aborting() -> anyh
|
||||
.await?;
|
||||
|
||||
let goal = state_db
|
||||
.thread_goals()
|
||||
.get_thread_goal(sess.conversation_id)
|
||||
.await?
|
||||
.expect("goal should remain persisted after follow-up accounting");
|
||||
@@ -8610,6 +8646,57 @@ async fn budget_limited_accounting_steers_active_turn_without_aborting() -> anyh
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn usage_limit_runtime_stops_active_goal_and_prevents_idle_continuation() -> anyhow::Result<()>
|
||||
{
|
||||
let (sess, tc, _rx, _codex_home) = make_goal_session_and_context_with_rx().await;
|
||||
sess.set_thread_goal(
|
||||
tc.as_ref(),
|
||||
SetGoalRequest {
|
||||
objective: Some("Keep improving the benchmark".to_string()),
|
||||
status: None,
|
||||
token_budget: Some(Some(50)),
|
||||
},
|
||||
)
|
||||
.await?;
|
||||
sess.goal_runtime_apply(GoalRuntimeEvent::TurnStarted {
|
||||
turn_context: tc.as_ref(),
|
||||
token_usage: TokenUsage::default(),
|
||||
})
|
||||
.await?;
|
||||
sess.spawn_task(
|
||||
Arc::clone(&tc),
|
||||
Vec::new(),
|
||||
NeverEndingTask {
|
||||
kind: TaskKind::Regular,
|
||||
listen_to_cancellation_token: false,
|
||||
},
|
||||
)
|
||||
.await;
|
||||
set_total_token_usage(&sess, post_goal_token_usage()).await;
|
||||
|
||||
sess.goal_runtime_apply(GoalRuntimeEvent::UsageLimitReached {
|
||||
turn_context: tc.as_ref(),
|
||||
})
|
||||
.await?;
|
||||
|
||||
let state_db = goal_test_state_db(sess.as_ref()).await?;
|
||||
let goal = state_db
|
||||
.thread_goals()
|
||||
.get_thread_goal(sess.conversation_id)
|
||||
.await?
|
||||
.expect("goal should remain persisted after usage limiting");
|
||||
assert_eq!(codex_state::ThreadGoalStatus::UsageLimited, goal.status);
|
||||
assert_eq!(70, goal.tokens_used);
|
||||
|
||||
sess.abort_all_tasks(TurnAbortReason::Replaced).await;
|
||||
sess.goal_runtime_apply(GoalRuntimeEvent::MaybeContinueIfIdle)
|
||||
.await?;
|
||||
assert!(sess.active_turn.lock().await.is_none());
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn external_goal_mutation_accounts_active_turn_before_status_change() -> anyhow::Result<()> {
|
||||
let (sess, tc, _rx, _codex_home) = make_goal_session_and_context_with_rx().await;
|
||||
@@ -8638,6 +8725,7 @@ async fn external_goal_mutation_accounts_active_turn_before_status_change() -> a
|
||||
|
||||
let state_db = goal_test_state_db(sess.as_ref()).await?;
|
||||
let goal = state_db
|
||||
.thread_goals()
|
||||
.get_thread_goal(sess.conversation_id)
|
||||
.await?
|
||||
.expect("goal should remain persisted");
|
||||
@@ -8646,6 +8734,7 @@ async fn external_goal_mutation_accounts_active_turn_before_status_change() -> a
|
||||
let previous_goal = goal.clone();
|
||||
let goal_id = goal.goal_id.clone();
|
||||
let updated_goal = state_db
|
||||
.thread_goals()
|
||||
.update_thread_goal(
|
||||
sess.conversation_id,
|
||||
codex_state::ThreadGoalUpdate {
|
||||
@@ -8667,6 +8756,7 @@ async fn external_goal_mutation_accounts_active_turn_before_status_change() -> a
|
||||
|
||||
assert!(sess.active_turn.lock().await.is_some());
|
||||
let goal = state_db
|
||||
.thread_goals()
|
||||
.get_thread_goal(sess.conversation_id)
|
||||
.await?
|
||||
.expect("goal should remain persisted");
|
||||
@@ -8693,6 +8783,7 @@ async fn external_objective_change_steers_active_turn() -> anyhow::Result<()> {
|
||||
|
||||
let state_db = goal_test_state_db(sess.as_ref()).await?;
|
||||
let old_goal = state_db
|
||||
.thread_goals()
|
||||
.replace_thread_goal(
|
||||
sess.conversation_id,
|
||||
"Keep improving the benchmark",
|
||||
@@ -8701,6 +8792,7 @@ async fn external_objective_change_steers_active_turn() -> anyhow::Result<()> {
|
||||
)
|
||||
.await?;
|
||||
let new_goal = state_db
|
||||
.thread_goals()
|
||||
.replace_thread_goal(
|
||||
sess.conversation_id,
|
||||
"Write a concise benchmark summary",
|
||||
@@ -8717,7 +8809,7 @@ async fn external_objective_change_steers_active_turn() -> anyhow::Result<()> {
|
||||
})
|
||||
.await?;
|
||||
|
||||
let pending_input = sess.get_pending_input().await;
|
||||
let pending_input = sess.input_queue.get_pending_input(&sess.active_turn).await;
|
||||
assert!(
|
||||
pending_input.iter().any(|item| {
|
||||
matches!(
|
||||
@@ -8758,6 +8850,7 @@ async fn external_active_goal_set_marks_current_turn_for_accounting() -> anyhow:
|
||||
|
||||
let state_db = goal_test_state_db(sess.as_ref()).await?;
|
||||
let goal = state_db
|
||||
.thread_goals()
|
||||
.replace_thread_goal(
|
||||
sess.conversation_id,
|
||||
"Keep improving the benchmark",
|
||||
@@ -8791,6 +8884,7 @@ async fn external_active_goal_set_marks_current_turn_for_accounting() -> anyhow:
|
||||
.await?;
|
||||
|
||||
let goal = state_db
|
||||
.thread_goals()
|
||||
.get_thread_goal(sess.conversation_id)
|
||||
.await?
|
||||
.expect("goal should remain persisted");
|
||||
@@ -8890,6 +8984,7 @@ async fn completed_goal_accounts_current_turn_tokens_before_tool_response() -> a
|
||||
)
|
||||
.await?;
|
||||
let persisted_goal = state_db
|
||||
.thread_goals()
|
||||
.get_thread_goal(test.session_configured.thread_id)
|
||||
.await?
|
||||
.expect("goal should be persisted");
|
||||
@@ -8922,19 +9017,26 @@ async fn queue_only_mailbox_mail_waits_for_next_turn_after_answer_boundary() {
|
||||
)
|
||||
.await;
|
||||
|
||||
sess.defer_mailbox_delivery_to_next_turn(&tc.sub_id).await;
|
||||
sess.enqueue_mailbox_communication(communication.clone());
|
||||
sess.input_queue
|
||||
.defer_mailbox_delivery_to_next_turn(&sess.active_turn, &tc.sub_id)
|
||||
.await;
|
||||
sess.input_queue
|
||||
.enqueue_mailbox_communication(communication.clone())
|
||||
.await;
|
||||
|
||||
assert!(
|
||||
!sess.has_pending_input().await,
|
||||
!sess.input_queue.has_pending_input(&sess.active_turn).await,
|
||||
"queue-only mailbox mail should stay buffered once the current turn emitted its answer"
|
||||
);
|
||||
assert_eq!(sess.get_pending_input().await, Vec::new());
|
||||
assert_eq!(
|
||||
sess.input_queue.get_pending_input(&sess.active_turn).await,
|
||||
Vec::new()
|
||||
);
|
||||
|
||||
sess.abort_all_tasks(TurnAbortReason::Replaced).await;
|
||||
|
||||
assert_eq!(
|
||||
sess.get_pending_input().await,
|
||||
sess.input_queue.get_pending_input(&sess.active_turn).await,
|
||||
vec![communication.to_response_input_item()],
|
||||
);
|
||||
}
|
||||
@@ -8952,23 +9054,27 @@ async fn trigger_turn_mailbox_mail_waits_for_next_turn_after_answer_boundary() {
|
||||
)
|
||||
.await;
|
||||
|
||||
sess.defer_mailbox_delivery_to_next_turn(&tc.sub_id).await;
|
||||
sess.enqueue_mailbox_communication(InterAgentCommunication::new(
|
||||
AgentPath::try_from("/root/worker").expect("worker path should parse"),
|
||||
AgentPath::root(),
|
||||
Vec::new(),
|
||||
"late trigger update".to_string(),
|
||||
/*trigger_turn*/ true,
|
||||
));
|
||||
sess.input_queue
|
||||
.defer_mailbox_delivery_to_next_turn(&sess.active_turn, &tc.sub_id)
|
||||
.await;
|
||||
sess.input_queue
|
||||
.enqueue_mailbox_communication(InterAgentCommunication::new(
|
||||
AgentPath::try_from("/root/worker").expect("worker path should parse"),
|
||||
AgentPath::root(),
|
||||
Vec::new(),
|
||||
"late trigger update".to_string(),
|
||||
/*trigger_turn*/ true,
|
||||
))
|
||||
.await;
|
||||
|
||||
assert!(
|
||||
!sess.has_pending_input().await,
|
||||
!sess.input_queue.has_pending_input(&sess.active_turn).await,
|
||||
"trigger-turn mailbox mail should not extend the current turn after its answer boundary"
|
||||
);
|
||||
|
||||
sess.abort_all_tasks(TurnAbortReason::Replaced).await;
|
||||
|
||||
assert!(sess.has_trigger_turn_mailbox_items().await);
|
||||
assert!(sess.input_queue.has_trigger_turn_mailbox_items().await);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -8991,8 +9097,12 @@ async fn steered_input_reopens_mailbox_delivery_for_current_turn() {
|
||||
)
|
||||
.await;
|
||||
|
||||
sess.defer_mailbox_delivery_to_next_turn(&tc.sub_id).await;
|
||||
sess.enqueue_mailbox_communication(communication.clone());
|
||||
sess.input_queue
|
||||
.defer_mailbox_delivery_to_next_turn(&sess.active_turn, &tc.sub_id)
|
||||
.await;
|
||||
sess.input_queue
|
||||
.enqueue_mailbox_communication(communication.clone())
|
||||
.await;
|
||||
sess.steer_input(
|
||||
vec![UserInput::Text {
|
||||
text: "follow up".to_string(),
|
||||
@@ -9005,7 +9115,7 @@ async fn steered_input_reopens_mailbox_delivery_for_current_turn() {
|
||||
.expect("steered input should be accepted");
|
||||
|
||||
assert_eq!(
|
||||
sess.get_pending_input().await,
|
||||
sess.input_queue.get_pending_input(&sess.active_turn).await,
|
||||
vec![
|
||||
ResponseInputItem::from(vec![UserInput::Text {
|
||||
text: "follow up".to_string(),
|
||||
@@ -9036,8 +9146,12 @@ async fn stale_defer_mailbox_delivery_does_not_override_steered_input() {
|
||||
)
|
||||
.await;
|
||||
|
||||
sess.defer_mailbox_delivery_to_next_turn(&tc.sub_id).await;
|
||||
sess.enqueue_mailbox_communication(communication.clone());
|
||||
sess.input_queue
|
||||
.defer_mailbox_delivery_to_next_turn(&sess.active_turn, &tc.sub_id)
|
||||
.await;
|
||||
sess.input_queue
|
||||
.enqueue_mailbox_communication(communication.clone())
|
||||
.await;
|
||||
sess.steer_input(
|
||||
vec![UserInput::Text {
|
||||
text: "follow up".to_string(),
|
||||
@@ -9049,10 +9163,12 @@ async fn stale_defer_mailbox_delivery_does_not_override_steered_input() {
|
||||
.await
|
||||
.expect("steered input should be accepted");
|
||||
|
||||
sess.defer_mailbox_delivery_to_next_turn(&tc.sub_id).await;
|
||||
sess.input_queue
|
||||
.defer_mailbox_delivery_to_next_turn(&sess.active_turn, &tc.sub_id)
|
||||
.await;
|
||||
|
||||
assert_eq!(
|
||||
sess.get_pending_input().await,
|
||||
sess.input_queue.get_pending_input(&sess.active_turn).await,
|
||||
vec![
|
||||
ResponseInputItem::from(vec![UserInput::Text {
|
||||
text: "follow up".to_string(),
|
||||
@@ -9083,8 +9199,12 @@ async fn tool_calls_reopen_mailbox_delivery_for_current_turn() {
|
||||
)
|
||||
.await;
|
||||
|
||||
sess.defer_mailbox_delivery_to_next_turn(&tc.sub_id).await;
|
||||
sess.enqueue_mailbox_communication(communication.clone());
|
||||
sess.input_queue
|
||||
.defer_mailbox_delivery_to_next_turn(&sess.active_turn, &tc.sub_id)
|
||||
.await;
|
||||
sess.input_queue
|
||||
.enqueue_mailbox_communication(communication.clone())
|
||||
.await;
|
||||
|
||||
let item = ResponseItem::FunctionCall {
|
||||
id: None,
|
||||
@@ -9108,7 +9228,7 @@ async fn tool_calls_reopen_mailbox_delivery_for_current_turn() {
|
||||
assert!(output.needs_follow_up);
|
||||
assert!(output.tool_future.is_some());
|
||||
assert_eq!(
|
||||
sess.get_pending_input().await,
|
||||
sess.input_queue.get_pending_input(&sess.active_turn).await,
|
||||
vec![communication.to_response_input_item()],
|
||||
);
|
||||
}
|
||||
@@ -9524,7 +9644,121 @@ async fn update_goal_tool_rejects_pausing_goal() {
|
||||
};
|
||||
assert_eq!(
|
||||
output,
|
||||
"update_goal can only mark the existing goal complete; pause, resume, and budget-limited status changes are controlled by the user or system"
|
||||
"update_goal can only mark the existing goal complete or blocked; pause, resume, budget-limited, and usage-limited status changes are controlled by the user or system"
|
||||
);
|
||||
|
||||
let goal = session
|
||||
.get_thread_goal()
|
||||
.await
|
||||
.expect("read thread goal")
|
||||
.expect("goal should still exist");
|
||||
assert_eq!(goal.status, ThreadGoalStatus::Active);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn update_goal_tool_marks_goal_blocked() {
|
||||
let (session, turn_context, _rx, _codex_home) = make_goal_session_and_context_with_rx().await;
|
||||
let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new()));
|
||||
let create_handler = CreateGoalHandler;
|
||||
let update_handler = UpdateGoalHandler;
|
||||
|
||||
create_handler
|
||||
.handle(ToolInvocation {
|
||||
session: Arc::clone(&session),
|
||||
turn: Arc::clone(&turn_context),
|
||||
cancellation_token: CancellationToken::new(),
|
||||
tracker: Arc::clone(&tracker),
|
||||
call_id: "create-goal".to_string(),
|
||||
tool_name: codex_tools::ToolName::plain("create_goal"),
|
||||
source: ToolCallSource::Direct,
|
||||
payload: ToolPayload::Function {
|
||||
arguments: serde_json::json!({
|
||||
"objective": "Keep the watcher alive",
|
||||
"token_budget": 123,
|
||||
})
|
||||
.to_string(),
|
||||
},
|
||||
})
|
||||
.await
|
||||
.expect("initial create_goal should succeed");
|
||||
|
||||
update_handler
|
||||
.handle(ToolInvocation {
|
||||
session: Arc::clone(&session),
|
||||
turn: Arc::clone(&turn_context),
|
||||
cancellation_token: CancellationToken::new(),
|
||||
tracker,
|
||||
call_id: "block-goal".to_string(),
|
||||
tool_name: codex_tools::ToolName::plain("update_goal"),
|
||||
source: ToolCallSource::Direct,
|
||||
payload: ToolPayload::Function {
|
||||
arguments: serde_json::json!({
|
||||
"status": "blocked",
|
||||
})
|
||||
.to_string(),
|
||||
},
|
||||
})
|
||||
.await
|
||||
.expect("update_goal should mark the goal blocked");
|
||||
|
||||
let goal = session
|
||||
.get_thread_goal()
|
||||
.await
|
||||
.expect("read thread goal")
|
||||
.expect("goal should still exist");
|
||||
assert_eq!(goal.status, ThreadGoalStatus::Blocked);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn update_goal_tool_rejects_usage_limited_goal() {
|
||||
let (session, turn_context, _rx, _codex_home) = make_goal_session_and_context_with_rx().await;
|
||||
let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new()));
|
||||
let create_handler = CreateGoalHandler;
|
||||
let update_handler = UpdateGoalHandler;
|
||||
|
||||
create_handler
|
||||
.handle(ToolInvocation {
|
||||
session: Arc::clone(&session),
|
||||
turn: Arc::clone(&turn_context),
|
||||
cancellation_token: CancellationToken::new(),
|
||||
tracker: Arc::clone(&tracker),
|
||||
call_id: "create-goal".to_string(),
|
||||
tool_name: codex_tools::ToolName::plain("create_goal"),
|
||||
source: ToolCallSource::Direct,
|
||||
payload: ToolPayload::Function {
|
||||
arguments: serde_json::json!({
|
||||
"objective": "Keep the watcher alive",
|
||||
})
|
||||
.to_string(),
|
||||
},
|
||||
})
|
||||
.await
|
||||
.expect("initial create_goal should succeed");
|
||||
|
||||
let response = update_handler
|
||||
.handle(ToolInvocation {
|
||||
session: Arc::clone(&session),
|
||||
turn: Arc::clone(&turn_context),
|
||||
cancellation_token: CancellationToken::new(),
|
||||
tracker,
|
||||
call_id: "usage-limit-goal".to_string(),
|
||||
tool_name: codex_tools::ToolName::plain("update_goal"),
|
||||
source: ToolCallSource::Direct,
|
||||
payload: ToolPayload::Function {
|
||||
arguments: serde_json::json!({
|
||||
"status": "usageLimited",
|
||||
})
|
||||
.to_string(),
|
||||
},
|
||||
})
|
||||
.await;
|
||||
|
||||
let Err(FunctionCallError::RespondToModel(output)) = response else {
|
||||
panic!("expected update_goal to reject usage-limiting a goal");
|
||||
};
|
||||
assert_eq!(
|
||||
output,
|
||||
"update_goal can only mark the existing goal complete or blocked; pause, resume, budget-limited, and usage-limited status changes are controlled by the user or system"
|
||||
);
|
||||
|
||||
let goal = session
|
||||
|
||||
@@ -9,7 +9,6 @@ use crate::build_skill_injections;
|
||||
use crate::client::ModelClientSession;
|
||||
use crate::client_common::Prompt;
|
||||
use crate::client_common::ResponseEvent;
|
||||
use crate::collect_env_var_dependencies;
|
||||
use crate::collect_explicit_skill_mentions;
|
||||
use crate::compact::InitialContextInjection;
|
||||
use crate::compact::collect_user_messages;
|
||||
@@ -20,6 +19,7 @@ use crate::compact_remote_v2::run_inline_remote_auto_compact_task as run_inline_
|
||||
use crate::connectors;
|
||||
use crate::context::ContextualUserFragment;
|
||||
use crate::feedback_tags;
|
||||
use crate::goals::GoalRuntimeEvent;
|
||||
use crate::hook_runtime::PendingInputHookDisposition;
|
||||
use crate::hook_runtime::emit_hook_completed_events;
|
||||
use crate::hook_runtime::inspect_pending_input;
|
||||
@@ -39,7 +39,6 @@ use crate::mentions::collect_explicit_plugin_mentions;
|
||||
use crate::mentions::collect_tool_mentions_from_messages;
|
||||
use crate::parse_turn_item;
|
||||
use crate::plugins::build_plugin_injections;
|
||||
use crate::resolve_skill_dependencies_for_turn;
|
||||
use crate::session::PreviousTurnSettings;
|
||||
use crate::session::session::Session;
|
||||
use crate::session::turn_context::TurnContext;
|
||||
@@ -71,6 +70,7 @@ use codex_analytics::build_track_events_context;
|
||||
use codex_async_utils::OrCancelExt;
|
||||
use codex_features::Feature;
|
||||
use codex_git_utils::get_git_repo_root;
|
||||
use codex_git_utils::get_git_repo_root_with_fs;
|
||||
use codex_hooks::HookEvent;
|
||||
use codex_hooks::HookEventAfterAgent;
|
||||
use codex_hooks::HookPayload;
|
||||
@@ -146,7 +146,7 @@ pub(crate) async fn run_turn(
|
||||
prewarmed_client_session: Option<ModelClientSession>,
|
||||
cancellation_token: CancellationToken,
|
||||
) -> Option<String> {
|
||||
if input.is_empty() && !sess.has_pending_input().await {
|
||||
if input.is_empty() && !sess.input_queue.has_pending_input(&sess.active_turn).await {
|
||||
return None;
|
||||
}
|
||||
|
||||
@@ -158,16 +158,18 @@ pub(crate) async fn run_turn(
|
||||
// new user message are recorded. Estimate pending incoming items (context
|
||||
// diffs/full reinjection + user input) and trigger compaction preemptively
|
||||
// when they would push the thread over the compaction threshold.
|
||||
let pre_sampling_compact =
|
||||
match run_pre_sampling_compact(&sess, &turn_context, &mut client_session).await {
|
||||
Ok(pre_sampling_compact) => pre_sampling_compact,
|
||||
Err(_) => {
|
||||
error!("Failed to run pre-sampling compact");
|
||||
return None;
|
||||
}
|
||||
};
|
||||
if pre_sampling_compact.reset_client_session {
|
||||
client_session.reset_websocket_session();
|
||||
if let Err(err) = run_pre_sampling_compact(&sess, &turn_context, &mut client_session).await {
|
||||
if err.to_codex_protocol_error() == CodexErrorInfo::UsageLimitExceeded
|
||||
&& let Err(err) = sess
|
||||
.goal_runtime_apply(GoalRuntimeEvent::UsageLimitReached {
|
||||
turn_context: turn_context.as_ref(),
|
||||
})
|
||||
.await
|
||||
{
|
||||
warn!("failed to usage-limit active goal after usage-limit error: {err}");
|
||||
}
|
||||
error!("Failed to run pre-sampling compact");
|
||||
return None;
|
||||
}
|
||||
|
||||
let skills_outcome = Some(turn_context.turn_skills.outcome.as_ref());
|
||||
@@ -230,15 +232,6 @@ pub(crate) async fn run_turn(
|
||||
&connector_slug_counts,
|
||||
)
|
||||
});
|
||||
let config = turn_context.config.clone();
|
||||
if config
|
||||
.features
|
||||
.enabled(Feature::SkillEnvVarDependencyPrompt)
|
||||
{
|
||||
let env_var_dependencies = collect_env_var_dependencies(&mentioned_skills);
|
||||
resolve_skill_dependencies_for_turn(&sess, &turn_context, &env_var_dependencies).await;
|
||||
}
|
||||
|
||||
maybe_prompt_and_install_mcp_dependencies(
|
||||
sess.as_ref(),
|
||||
turn_context.as_ref(),
|
||||
@@ -370,8 +363,17 @@ pub(crate) async fn run_turn(
|
||||
// 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)]
|
||||
let display_root = get_git_repo_root(turn_context.cwd.as_path())
|
||||
.unwrap_or_else(|| turn_context.cwd.clone().into_path_buf());
|
||||
let display_root = match turn_context.environments.primary() {
|
||||
Some(turn_environment) => get_git_repo_root_with_fs(
|
||||
turn_environment.environment.get_filesystem().as_ref(),
|
||||
&turn_environment.cwd,
|
||||
)
|
||||
.await
|
||||
.unwrap_or_else(|| turn_environment.cwd.clone())
|
||||
.into_path_buf(),
|
||||
None => get_git_repo_root(turn_context.cwd.as_path())
|
||||
.unwrap_or_else(|| turn_context.cwd.clone().into_path_buf()),
|
||||
};
|
||||
let turn_diff_tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::with_display_root(
|
||||
display_root,
|
||||
)));
|
||||
@@ -393,7 +395,7 @@ pub(crate) async fn run_turn(
|
||||
// submitted through the UI while the model was running. Though the UI
|
||||
// may support this, the model might not.
|
||||
let pending_input = if can_drain_pending_input {
|
||||
sess.get_pending_input().await
|
||||
sess.input_queue.get_pending_input(&sess.active_turn).await
|
||||
} else {
|
||||
Vec::new()
|
||||
};
|
||||
@@ -414,7 +416,10 @@ pub(crate) async fn run_turn(
|
||||
} => {
|
||||
let remaining_pending_input = pending_input_iter.collect::<Vec<_>>();
|
||||
if !remaining_pending_input.is_empty() {
|
||||
let _ = sess.prepend_pending_input(remaining_pending_input).await;
|
||||
let _ = sess
|
||||
.input_queue
|
||||
.prepend_pending_input(&sess.active_turn, remaining_pending_input)
|
||||
.await;
|
||||
requeued_pending_input = true;
|
||||
}
|
||||
blocked_pending_input_contexts = additional_contexts;
|
||||
@@ -474,7 +479,7 @@ pub(crate) async fn run_turn(
|
||||
last_agent_message: sampling_request_last_agent_message,
|
||||
} = sampling_request_output;
|
||||
can_drain_pending_input = true;
|
||||
let has_pending_input = sess.has_pending_input().await;
|
||||
let has_pending_input = sess.input_queue.has_pending_input(&sess.active_turn).await;
|
||||
let needs_follow_up = model_needs_follow_up || has_pending_input;
|
||||
let total_usage_tokens = sess.get_total_token_usage().await;
|
||||
let token_limit_reached = total_usage_tokens >= auto_compact_limit;
|
||||
@@ -496,7 +501,7 @@ pub(crate) async fn run_turn(
|
||||
|
||||
// as long as compaction works well in getting us way below the token limit, we shouldn't worry about being in an infinite loop.
|
||||
if token_limit_reached && needs_follow_up {
|
||||
let reset_client_session = match run_auto_compact(
|
||||
match run_auto_compact(
|
||||
&sess,
|
||||
&turn_context,
|
||||
&mut client_session,
|
||||
@@ -506,12 +511,22 @@ pub(crate) async fn run_turn(
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(reset_client_session) => reset_client_session,
|
||||
Err(_) => return None,
|
||||
Ok(()) => {}
|
||||
Err(err) => {
|
||||
if err.to_codex_protocol_error() == CodexErrorInfo::UsageLimitExceeded
|
||||
&& let Err(err) = sess
|
||||
.goal_runtime_apply(GoalRuntimeEvent::UsageLimitReached {
|
||||
turn_context: turn_context.as_ref(),
|
||||
})
|
||||
.await
|
||||
{
|
||||
warn!(
|
||||
"failed to usage-limit active goal after usage-limit error: {err}"
|
||||
);
|
||||
}
|
||||
return None;
|
||||
}
|
||||
};
|
||||
if reset_client_session {
|
||||
client_session.reset_websocket_session();
|
||||
}
|
||||
can_drain_pending_input = !model_needs_follow_up;
|
||||
continue;
|
||||
}
|
||||
@@ -663,6 +678,15 @@ pub(crate) async fn run_turn(
|
||||
}
|
||||
Err(e) => {
|
||||
info!("Turn error: {e:#}");
|
||||
if e.to_codex_protocol_error() == CodexErrorInfo::UsageLimitExceeded
|
||||
&& let Err(err) = sess
|
||||
.goal_runtime_apply(GoalRuntimeEvent::UsageLimitReached {
|
||||
turn_context: turn_context.as_ref(),
|
||||
})
|
||||
.await
|
||||
{
|
||||
warn!("failed to usage-limit active goal after usage-limit error: {err}");
|
||||
}
|
||||
let event = EventMsg::Error(e.to_error_event(/*message_prefix*/ None));
|
||||
sess.send_event(&turn_context, event).await;
|
||||
// let the user continue the conversation
|
||||
@@ -722,24 +746,19 @@ async fn track_turn_resolved_config_analytics(
|
||||
});
|
||||
}
|
||||
|
||||
struct PreSamplingCompactResult {
|
||||
reset_client_session: bool,
|
||||
}
|
||||
|
||||
async fn run_pre_sampling_compact(
|
||||
sess: &Arc<Session>,
|
||||
turn_context: &Arc<TurnContext>,
|
||||
client_session: &mut ModelClientSession,
|
||||
) -> CodexResult<PreSamplingCompactResult> {
|
||||
) -> CodexResult<()> {
|
||||
let total_usage_tokens_before_compaction = sess.get_total_token_usage().await;
|
||||
let mut pre_sampling_compacted = maybe_run_previous_model_inline_compact(
|
||||
maybe_run_previous_model_inline_compact(
|
||||
sess,
|
||||
turn_context,
|
||||
client_session,
|
||||
total_usage_tokens_before_compaction,
|
||||
)
|
||||
.await?;
|
||||
let mut reset_client_session = pre_sampling_compacted;
|
||||
let total_usage_tokens = sess.get_total_token_usage().await;
|
||||
let auto_compact_limit = turn_context
|
||||
.model_info
|
||||
@@ -747,7 +766,7 @@ async fn run_pre_sampling_compact(
|
||||
.unwrap_or(i64::MAX);
|
||||
// Compact if the total usage tokens are greater than the auto compact limit
|
||||
if total_usage_tokens >= auto_compact_limit {
|
||||
reset_client_session |= run_auto_compact(
|
||||
run_auto_compact(
|
||||
sess,
|
||||
turn_context,
|
||||
client_session,
|
||||
@@ -756,27 +775,22 @@ async fn run_pre_sampling_compact(
|
||||
CompactionPhase::PreTurn,
|
||||
)
|
||||
.await?;
|
||||
pre_sampling_compacted = true;
|
||||
}
|
||||
Ok(PreSamplingCompactResult {
|
||||
reset_client_session: pre_sampling_compacted && reset_client_session,
|
||||
})
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Runs pre-sampling compaction against the previous model when switching to a smaller
|
||||
/// context-window model.
|
||||
///
|
||||
/// Returns `Ok(true)` when compaction ran successfully, `Ok(false)` when compaction was skipped
|
||||
/// because the model/context-window preconditions were not met, and `Err(_)` only when compaction
|
||||
/// was attempted and failed.
|
||||
/// Returns `Err(_)` only when compaction was attempted and failed.
|
||||
async fn maybe_run_previous_model_inline_compact(
|
||||
sess: &Arc<Session>,
|
||||
turn_context: &Arc<TurnContext>,
|
||||
client_session: &mut ModelClientSession,
|
||||
total_usage_tokens: i64,
|
||||
) -> CodexResult<bool> {
|
||||
) -> CodexResult<()> {
|
||||
let Some(previous_turn_settings) = sess.previous_turn_settings().await else {
|
||||
return Ok(false);
|
||||
return Ok(());
|
||||
};
|
||||
let previous_model_turn_context = Arc::new(
|
||||
turn_context
|
||||
@@ -785,10 +799,10 @@ async fn maybe_run_previous_model_inline_compact(
|
||||
);
|
||||
|
||||
let Some(old_context_window) = previous_model_turn_context.model_context_window() else {
|
||||
return Ok(false);
|
||||
return Ok(());
|
||||
};
|
||||
let Some(new_context_window) = turn_context.model_context_window() else {
|
||||
return Ok(false);
|
||||
return Ok(());
|
||||
};
|
||||
let new_auto_compact_limit = turn_context
|
||||
.model_info
|
||||
@@ -798,7 +812,7 @@ async fn maybe_run_previous_model_inline_compact(
|
||||
&& previous_model_turn_context.model_info.slug != turn_context.model_info.slug
|
||||
&& old_context_window > new_context_window;
|
||||
if should_run {
|
||||
let _ = run_auto_compact(
|
||||
run_auto_compact(
|
||||
sess,
|
||||
&previous_model_turn_context,
|
||||
client_session,
|
||||
@@ -807,9 +821,8 @@ async fn maybe_run_previous_model_inline_compact(
|
||||
CompactionPhase::PreTurn,
|
||||
)
|
||||
.await?;
|
||||
return Ok(true);
|
||||
}
|
||||
Ok(false)
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn run_auto_compact(
|
||||
@@ -819,7 +832,7 @@ async fn run_auto_compact(
|
||||
initial_context_injection: InitialContextInjection,
|
||||
reason: CompactionReason,
|
||||
phase: CompactionPhase,
|
||||
) -> CodexResult<bool> {
|
||||
) -> CodexResult<()> {
|
||||
if should_use_remote_compact_task(turn_context.provider.info()) {
|
||||
if turn_context.features.enabled(Feature::RemoteCompactionV2) {
|
||||
run_inline_remote_auto_compact_task_v2(
|
||||
@@ -831,7 +844,7 @@ async fn run_auto_compact(
|
||||
phase,
|
||||
)
|
||||
.await?;
|
||||
return Ok(false);
|
||||
return Ok(());
|
||||
}
|
||||
run_inline_remote_auto_compact_task(
|
||||
Arc::clone(sess),
|
||||
@@ -851,7 +864,7 @@ async fn run_auto_compact(
|
||||
)
|
||||
.await?;
|
||||
}
|
||||
Ok(true)
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub(super) fn collect_explicit_app_ids_from_skill_items(
|
||||
@@ -2023,7 +2036,7 @@ async fn try_run_sampling_request(
|
||||
}
|
||||
needs_follow_up |= output_result.needs_follow_up;
|
||||
// todo: remove before stabilizing multi-agent v2
|
||||
if preempt_for_mailbox_mail && sess.mailbox_rx.lock().await.has_pending() {
|
||||
if preempt_for_mailbox_mail && sess.input_queue.has_pending_mailbox_items().await {
|
||||
break Ok(SamplingRequestResult {
|
||||
needs_follow_up: true,
|
||||
last_agent_message,
|
||||
|
||||
@@ -220,6 +220,7 @@ impl TurnContext {
|
||||
.with_spawn_agent_usage_hint(config.multi_agent_v2.usage_hint_enabled)
|
||||
.with_spawn_agent_usage_hint_text(config.multi_agent_v2.usage_hint_text.clone())
|
||||
.with_hide_spawn_agent_metadata(config.multi_agent_v2.hide_spawn_agent_metadata)
|
||||
.with_multi_agent_v2_tool_namespace(config.multi_agent_v2.tool_namespace.clone())
|
||||
.with_multi_agent_v2_non_code_mode_only(config.multi_agent_v2.non_code_mode_only)
|
||||
.with_goal_tools_allowed(self.tools_config.goal_tools)
|
||||
.with_max_concurrent_threads_per_session(
|
||||
@@ -367,7 +368,6 @@ impl TurnContext {
|
||||
pub(crate) fn to_turn_context_item(&self) -> TurnContextItem {
|
||||
TurnContextItem {
|
||||
turn_id: Some(self.sub_id.clone()),
|
||||
trace_id: self.trace_id.clone(),
|
||||
#[allow(deprecated)]
|
||||
cwd: self.cwd.to_path_buf(),
|
||||
current_date: self.current_date.clone(),
|
||||
@@ -382,11 +382,7 @@ impl TurnContext {
|
||||
collaboration_mode: Some(self.collaboration_mode.clone()),
|
||||
realtime_active: Some(self.realtime_active),
|
||||
effort: self.reasoning_effort,
|
||||
summary: self.reasoning_summary,
|
||||
user_instructions: self.user_instructions.clone(),
|
||||
developer_instructions: self.developer_instructions.clone(),
|
||||
final_output_json_schema: self.final_output_json_schema.clone(),
|
||||
truncation_policy: Some(self.truncation_policy),
|
||||
summary: ReasoningSummaryConfig::Auto,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -539,6 +535,7 @@ impl Session {
|
||||
.with_spawn_agent_usage_hint(per_turn_config.multi_agent_v2.usage_hint_enabled)
|
||||
.with_spawn_agent_usage_hint_text(per_turn_config.multi_agent_v2.usage_hint_text.clone())
|
||||
.with_hide_spawn_agent_metadata(per_turn_config.multi_agent_v2.hide_spawn_agent_metadata)
|
||||
.with_multi_agent_v2_tool_namespace(per_turn_config.multi_agent_v2.tool_namespace.clone())
|
||||
.with_multi_agent_v2_non_code_mode_only(per_turn_config.multi_agent_v2.non_code_mode_only)
|
||||
.with_goal_tools_allowed(goal_tools_supported)
|
||||
.with_max_concurrent_threads_per_session(
|
||||
|
||||
@@ -1,8 +1,3 @@
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
use std::env;
|
||||
use std::sync::Arc;
|
||||
|
||||
use crate::config::Config;
|
||||
use crate::session::session::Session;
|
||||
use crate::session::turn_context::TurnContext;
|
||||
@@ -10,14 +5,9 @@ use codex_analytics::InvocationType;
|
||||
use codex_analytics::SkillInvocation;
|
||||
use codex_analytics::build_track_events_context;
|
||||
use codex_protocol::protocol::SkillScope;
|
||||
use codex_protocol::request_user_input::RequestUserInputArgs;
|
||||
use codex_protocol::request_user_input::RequestUserInputQuestion;
|
||||
use codex_protocol::request_user_input::RequestUserInputResponse;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_plugins::PluginSkillRoot;
|
||||
use tracing::warn;
|
||||
|
||||
pub use codex_core_skills::SkillDependencyInfo;
|
||||
pub use codex_core_skills::SkillError;
|
||||
pub use codex_core_skills::SkillLoadOutcome;
|
||||
pub use codex_core_skills::SkillMetadata;
|
||||
@@ -27,7 +17,6 @@ pub use codex_core_skills::SkillsLoadInput;
|
||||
pub use codex_core_skills::SkillsManager;
|
||||
pub use codex_core_skills::build_available_skills;
|
||||
pub use codex_core_skills::build_skill_name_counts;
|
||||
pub use codex_core_skills::collect_env_var_dependencies;
|
||||
pub use codex_core_skills::config_rules;
|
||||
pub use codex_core_skills::default_skill_metadata_budget;
|
||||
pub use codex_core_skills::detect_implicit_skill_invocation_for_command;
|
||||
@@ -56,121 +45,6 @@ pub(crate) fn skills_load_input_from_config(
|
||||
)
|
||||
}
|
||||
|
||||
pub(crate) async fn resolve_skill_dependencies_for_turn(
|
||||
sess: &Arc<Session>,
|
||||
turn_context: &Arc<TurnContext>,
|
||||
dependencies: &[SkillDependencyInfo],
|
||||
) {
|
||||
if dependencies.is_empty() {
|
||||
return;
|
||||
}
|
||||
|
||||
let existing_env = sess.dependency_env().await;
|
||||
let mut loaded_values = HashMap::new();
|
||||
let mut missing = Vec::new();
|
||||
let mut seen_names = HashSet::new();
|
||||
|
||||
for dependency in dependencies {
|
||||
let name = dependency.name.clone();
|
||||
if !seen_names.insert(name.clone()) || existing_env.contains_key(&name) {
|
||||
continue;
|
||||
}
|
||||
match env::var(&name) {
|
||||
Ok(value) => {
|
||||
loaded_values.insert(name.clone(), value);
|
||||
}
|
||||
Err(env::VarError::NotPresent) => {
|
||||
missing.push(dependency.clone());
|
||||
}
|
||||
Err(err) => {
|
||||
warn!("failed to read env var {name}: {err}");
|
||||
missing.push(dependency.clone());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if !loaded_values.is_empty() {
|
||||
sess.set_dependency_env(loaded_values).await;
|
||||
}
|
||||
|
||||
if !missing.is_empty() {
|
||||
request_skill_dependencies(sess, turn_context, &missing).await;
|
||||
}
|
||||
}
|
||||
|
||||
async fn request_skill_dependencies(
|
||||
sess: &Arc<Session>,
|
||||
turn_context: &Arc<TurnContext>,
|
||||
dependencies: &[SkillDependencyInfo],
|
||||
) {
|
||||
let questions = dependencies
|
||||
.iter()
|
||||
.map(|dependency| {
|
||||
let requirement = dependency.description.as_ref().map_or_else(
|
||||
|| {
|
||||
format!(
|
||||
"The skill \"{}\" requires \"{}\" to be set.",
|
||||
dependency.skill_name, dependency.name
|
||||
)
|
||||
},
|
||||
|description| {
|
||||
format!(
|
||||
"The skill \"{}\" requires \"{}\" to be set ({}).",
|
||||
dependency.skill_name, dependency.name, description
|
||||
)
|
||||
},
|
||||
);
|
||||
RequestUserInputQuestion {
|
||||
id: dependency.name.clone(),
|
||||
header: "Skill requires environment variable".to_string(),
|
||||
question: format!(
|
||||
"{requirement} This is an experimental internal feature. The value is stored in memory for this session only."
|
||||
),
|
||||
is_other: false,
|
||||
is_secret: true,
|
||||
options: None,
|
||||
}
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
if questions.is_empty() {
|
||||
return;
|
||||
}
|
||||
|
||||
let response = sess
|
||||
.request_user_input(
|
||||
turn_context,
|
||||
format!("skill-deps-{}", turn_context.sub_id),
|
||||
RequestUserInputArgs { questions },
|
||||
)
|
||||
.await
|
||||
.unwrap_or_else(|| RequestUserInputResponse {
|
||||
answers: HashMap::new(),
|
||||
});
|
||||
if response.answers.is_empty() {
|
||||
return;
|
||||
}
|
||||
|
||||
let mut values = HashMap::new();
|
||||
for (name, answer) in response.answers {
|
||||
let mut user_note = None;
|
||||
for entry in &answer.answers {
|
||||
if let Some(note) = entry.strip_prefix("user_note: ")
|
||||
&& !note.trim().is_empty()
|
||||
{
|
||||
user_note = Some(note.trim().to_string());
|
||||
}
|
||||
}
|
||||
if let Some(value) = user_note {
|
||||
values.insert(name, value);
|
||||
}
|
||||
}
|
||||
if values.is_empty() {
|
||||
return;
|
||||
}
|
||||
|
||||
sess.set_dependency_env(values).await;
|
||||
}
|
||||
|
||||
pub(crate) async fn maybe_emit_implicit_skill_invocation(
|
||||
sess: &Session,
|
||||
turn_context: &TurnContext,
|
||||
|
||||
@@ -3,7 +3,6 @@
|
||||
use codex_protocol::models::AdditionalPermissionProfile;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_sandboxing::policy_transforms::merge_permission_profiles;
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
|
||||
use crate::context_manager::ContextManager;
|
||||
@@ -22,7 +21,6 @@ pub(crate) struct SessionState {
|
||||
pub(crate) history: ContextManager,
|
||||
pub(crate) latest_rate_limits: Option<RateLimitSnapshot>,
|
||||
pub(crate) server_reasoning_included: bool,
|
||||
pub(crate) dependency_env: HashMap<String, String>,
|
||||
pub(crate) mcp_dependency_prompted: HashSet<String>,
|
||||
/// Settings used by the latest regular user turn, used for turn-to-turn
|
||||
/// model/realtime handling on subsequent regular turns (including full-context
|
||||
@@ -45,7 +43,6 @@ impl SessionState {
|
||||
history,
|
||||
latest_rate_limits: None,
|
||||
server_reasoning_included: false,
|
||||
dependency_env: HashMap::new(),
|
||||
mcp_dependency_prompted: HashSet::new(),
|
||||
previous_turn_settings: None,
|
||||
startup_prewarm: None,
|
||||
@@ -165,16 +162,6 @@ impl SessionState {
|
||||
self.mcp_dependency_prompted.clone()
|
||||
}
|
||||
|
||||
pub(crate) fn set_dependency_env(&mut self, values: HashMap<String, String>) {
|
||||
for (key, value) in values {
|
||||
self.dependency_env.insert(key, value);
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn dependency_env(&self) -> HashMap<String, String> {
|
||||
self.dependency_env.clone()
|
||||
}
|
||||
|
||||
pub(crate) fn set_session_startup_prewarm(
|
||||
&mut self,
|
||||
startup_prewarm: SessionStartupPrewarmHandle,
|
||||
|
||||
@@ -11,7 +11,6 @@ use tokio_util::task::AbortOnDropHandle;
|
||||
|
||||
use codex_extension_api::ExtensionData;
|
||||
use codex_protocol::dynamic_tools::DynamicToolResponse;
|
||||
use codex_protocol::models::ResponseInputItem;
|
||||
use codex_protocol::request_permissions::RequestPermissionProfile;
|
||||
use codex_protocol::request_permissions::RequestPermissionsResponse;
|
||||
use codex_protocol::request_user_input::RequestUserInputResponse;
|
||||
@@ -20,6 +19,7 @@ use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use rmcp::model::RequestId;
|
||||
use tokio::sync::oneshot;
|
||||
|
||||
use crate::session::TurnInputQueue;
|
||||
use crate::session::turn_context::TurnContext;
|
||||
use crate::tasks::AnySessionTask;
|
||||
use codex_protocol::models::AdditionalPermissionProfile;
|
||||
@@ -115,7 +115,7 @@ pub(crate) struct TurnState {
|
||||
pending_user_input: HashMap<String, oneshot::Sender<RequestUserInputResponse>>,
|
||||
pending_elicitations: HashMap<(String, RequestId), oneshot::Sender<ElicitationResponse>>,
|
||||
pending_dynamic_tools: HashMap<String, oneshot::Sender<DynamicToolResponse>>,
|
||||
pending_input: Vec<ResponseInputItem>,
|
||||
pub(crate) pending_input: TurnInputQueue,
|
||||
mailbox_delivery_phase: MailboxDeliveryPhase,
|
||||
granted_permissions: Option<AdditionalPermissionProfile>,
|
||||
strict_auto_review_enabled: bool,
|
||||
@@ -146,13 +146,12 @@ impl TurnState {
|
||||
self.pending_approvals.remove(key)
|
||||
}
|
||||
|
||||
pub(crate) fn clear_pending(&mut self) {
|
||||
pub(crate) fn clear_pending_waiters(&mut self) {
|
||||
self.pending_approvals.clear();
|
||||
self.pending_request_permissions.clear();
|
||||
self.pending_user_input.clear();
|
||||
self.pending_elicitations.clear();
|
||||
self.pending_dynamic_tools.clear();
|
||||
self.pending_input.clear();
|
||||
}
|
||||
|
||||
pub(crate) fn insert_pending_request_permissions(
|
||||
@@ -220,33 +219,6 @@ impl TurnState {
|
||||
self.pending_dynamic_tools.remove(key)
|
||||
}
|
||||
|
||||
pub(crate) fn push_pending_input(&mut self, input: ResponseInputItem) {
|
||||
self.pending_input.push(input);
|
||||
}
|
||||
|
||||
pub(crate) fn prepend_pending_input(&mut self, mut input: Vec<ResponseInputItem>) {
|
||||
if input.is_empty() {
|
||||
return;
|
||||
}
|
||||
|
||||
input.append(&mut self.pending_input);
|
||||
self.pending_input = input;
|
||||
}
|
||||
|
||||
pub(crate) fn take_pending_input(&mut self) -> Vec<ResponseInputItem> {
|
||||
if self.pending_input.is_empty() {
|
||||
Vec::with_capacity(0)
|
||||
} else {
|
||||
let mut ret = Vec::new();
|
||||
std::mem::swap(&mut ret, &mut self.pending_input);
|
||||
ret
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn has_pending_input(&self) -> bool {
|
||||
!self.pending_input.is_empty()
|
||||
}
|
||||
|
||||
pub(crate) fn accept_mailbox_delivery_for_current_turn(&mut self) {
|
||||
self.set_mailbox_delivery_phase(MailboxDeliveryPhase::CurrentTurn);
|
||||
}
|
||||
@@ -276,11 +248,3 @@ impl TurnState {
|
||||
self.strict_auto_review_enabled
|
||||
}
|
||||
}
|
||||
|
||||
impl ActiveTurn {
|
||||
/// Clear any pending approvals and input buffered for the current turn.
|
||||
pub(crate) async fn clear_pending(&self) {
|
||||
let mut ts = self.turn_state.lock().await;
|
||||
ts.clear_pending();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -158,7 +158,8 @@ pub(crate) async fn record_completed_response_item_with_finalized_facts(
|
||||
|facts| facts.defers_mailbox_delivery_to_next_turn,
|
||||
);
|
||||
if defers_mailbox_delivery {
|
||||
sess.defer_mailbox_delivery_to_next_turn(&turn_context.sub_id)
|
||||
sess.input_queue
|
||||
.defer_mailbox_delivery_to_next_turn(&sess.active_turn, &turn_context.sub_id)
|
||||
.await;
|
||||
}
|
||||
mark_thread_memory_mode_polluted_if_external_context(sess, turn_context, item).await;
|
||||
@@ -351,7 +352,11 @@ pub(crate) async fn handle_output_item_done(
|
||||
// The model emitted a tool call; log it, persist the item immediately, and queue the tool execution.
|
||||
Ok(Some(call)) => {
|
||||
ctx.sess
|
||||
.accept_mailbox_delivery_for_current_turn(&ctx.turn_context.sub_id)
|
||||
.input_queue
|
||||
.accept_mailbox_delivery_for_current_turn(
|
||||
&ctx.sess.active_turn,
|
||||
&ctx.turn_context.sub_id,
|
||||
)
|
||||
.await;
|
||||
|
||||
let payload_preview = call.payload.log_payload().into_owned();
|
||||
|
||||
@@ -4,38 +4,44 @@ use codex_protocol::protocol::TurnAbortReason;
|
||||
use crate::session::session::Session;
|
||||
|
||||
impl Session {
|
||||
pub(super) fn emit_turn_start_lifecycle(&self, turn_store: &ExtensionData) {
|
||||
pub(super) async fn emit_turn_start_lifecycle(&self, turn_store: &ExtensionData) {
|
||||
for contributor in self.services.extensions.turn_lifecycle_contributors() {
|
||||
contributor.on_turn_start(codex_extension_api::TurnStartInput {
|
||||
session_store: &self.services.session_extension_data,
|
||||
thread_store: &self.services.thread_extension_data,
|
||||
turn_store,
|
||||
});
|
||||
contributor
|
||||
.on_turn_start(codex_extension_api::TurnStartInput {
|
||||
session_store: &self.services.session_extension_data,
|
||||
thread_store: &self.services.thread_extension_data,
|
||||
turn_store,
|
||||
})
|
||||
.await;
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn emit_turn_stop_lifecycle(&self, turn_store: &ExtensionData) {
|
||||
pub(super) async fn emit_turn_stop_lifecycle(&self, turn_store: &ExtensionData) {
|
||||
for contributor in self.services.extensions.turn_lifecycle_contributors() {
|
||||
contributor.on_turn_stop(codex_extension_api::TurnStopInput {
|
||||
session_store: &self.services.session_extension_data,
|
||||
thread_store: &self.services.thread_extension_data,
|
||||
turn_store,
|
||||
});
|
||||
contributor
|
||||
.on_turn_stop(codex_extension_api::TurnStopInput {
|
||||
session_store: &self.services.session_extension_data,
|
||||
thread_store: &self.services.thread_extension_data,
|
||||
turn_store,
|
||||
})
|
||||
.await;
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn emit_turn_abort_lifecycle(
|
||||
pub(super) async fn emit_turn_abort_lifecycle(
|
||||
&self,
|
||||
reason: TurnAbortReason,
|
||||
turn_store: &ExtensionData,
|
||||
) {
|
||||
for contributor in self.services.extensions.turn_lifecycle_contributors() {
|
||||
contributor.on_turn_abort(codex_extension_api::TurnAbortInput {
|
||||
reason: reason.clone(),
|
||||
session_store: &self.services.session_extension_data,
|
||||
thread_store: &self.services.thread_extension_data,
|
||||
turn_store,
|
||||
});
|
||||
contributor
|
||||
.on_turn_abort(codex_extension_api::TurnAbortInput {
|
||||
reason: reason.clone(),
|
||||
session_store: &self.services.session_extension_data,
|
||||
thread_store: &self.services.thread_extension_data,
|
||||
turn_store,
|
||||
})
|
||||
.await;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -347,25 +347,25 @@ impl Session {
|
||||
{
|
||||
warn!("failed to apply goal runtime turn-start event: {err}");
|
||||
}
|
||||
let queued_response_items = self.take_queued_response_items_for_next_turn().await;
|
||||
let mailbox_items = self.get_pending_input().await;
|
||||
let queued_response_items = self
|
||||
.input_queue
|
||||
.take_queued_response_items_for_next_turn()
|
||||
.await;
|
||||
let mailbox_items = self.input_queue.get_pending_input(&self.active_turn).await;
|
||||
let turn_state = {
|
||||
let mut active = self.active_turn.lock().await;
|
||||
let turn = active.get_or_insert_with(ActiveTurn::default);
|
||||
debug_assert!(turn.tasks.is_empty());
|
||||
Arc::clone(&turn.turn_state)
|
||||
};
|
||||
{
|
||||
let mut turn_state = turn_state.lock().await;
|
||||
turn_state.token_usage_at_turn_start = token_usage_at_turn_start;
|
||||
for item in queued_response_items {
|
||||
turn_state.push_pending_input(item);
|
||||
}
|
||||
for item in mailbox_items {
|
||||
turn_state.push_pending_input(item);
|
||||
}
|
||||
}
|
||||
self.emit_turn_start_lifecycle(turn_context.extension_data.as_ref());
|
||||
turn_state.lock().await.token_usage_at_turn_start = token_usage_at_turn_start;
|
||||
let mut pending_items = queued_response_items;
|
||||
pending_items.extend(mailbox_items);
|
||||
self.input_queue
|
||||
.extend_pending_input_for_turn_state(turn_state.as_ref(), pending_items)
|
||||
.await;
|
||||
self.emit_turn_start_lifecycle(turn_context.extension_data.as_ref())
|
||||
.await;
|
||||
|
||||
let turn_extension_data = Arc::clone(&turn_context.extension_data);
|
||||
let mut active = self.active_turn.lock().await;
|
||||
@@ -467,8 +467,11 @@ impl Session {
|
||||
self: &Arc<Self>,
|
||||
sub_id: String,
|
||||
) {
|
||||
if !self.has_queued_response_items_for_next_turn().await
|
||||
&& !self.has_trigger_turn_mailbox_items().await
|
||||
if !self
|
||||
.input_queue
|
||||
.has_queued_response_items_for_next_turn()
|
||||
.await
|
||||
&& !self.input_queue.has_trigger_turn_mailbox_items().await
|
||||
{
|
||||
return;
|
||||
}
|
||||
@@ -505,13 +508,13 @@ impl Session {
|
||||
}
|
||||
|
||||
if let Some(turn_context) = turn_context.as_deref() {
|
||||
self.emit_turn_abort_lifecycle(reason.clone(), turn_context.extension_data.as_ref());
|
||||
self.emit_turn_abort_lifecycle(reason.clone(), turn_context.extension_data.as_ref())
|
||||
.await;
|
||||
}
|
||||
if (aborted_turn || reason == TurnAbortReason::Interrupted)
|
||||
&& let Err(err) = self
|
||||
.goal_runtime_apply(GoalRuntimeEvent::TaskAborted {
|
||||
turn_context: turn_context.as_deref(),
|
||||
reason: reason.clone(),
|
||||
})
|
||||
.await
|
||||
{
|
||||
@@ -520,7 +523,7 @@ impl Session {
|
||||
if let Some(active_turn) = active_turn_to_clear {
|
||||
// Let interrupted tasks observe cancellation before dropping pending approvals, or an
|
||||
// in-flight approval wait can surface as a model-visible rejection before TurnAborted.
|
||||
active_turn.clear_pending().await;
|
||||
self.input_queue.clear_pending(&active_turn).await;
|
||||
}
|
||||
if reason == TurnAbortReason::Interrupted && aborted_turn {
|
||||
self.maybe_start_turn_for_pending_work().await;
|
||||
@@ -553,12 +556,12 @@ impl Session {
|
||||
self.handle_task_abort(task, reason.clone()).await;
|
||||
}
|
||||
if let Some(turn_context) = turn_context.as_deref() {
|
||||
self.emit_turn_abort_lifecycle(reason.clone(), turn_context.extension_data.as_ref());
|
||||
self.emit_turn_abort_lifecycle(reason.clone(), turn_context.extension_data.as_ref())
|
||||
.await;
|
||||
}
|
||||
if let Err(err) = self
|
||||
.goal_runtime_apply(GoalRuntimeEvent::TaskAborted {
|
||||
turn_context: turn_context.as_deref(),
|
||||
reason: reason.clone(),
|
||||
})
|
||||
.await
|
||||
{
|
||||
@@ -566,7 +569,7 @@ impl Session {
|
||||
}
|
||||
// Let interrupted tasks observe cancellation before dropping pending approvals, or an
|
||||
// in-flight approval wait can surface as a model-visible rejection before TurnAborted.
|
||||
active_turn.clear_pending().await;
|
||||
self.input_queue.clear_pending(&active_turn).await;
|
||||
|
||||
if reason == TurnAbortReason::Interrupted {
|
||||
self.maybe_start_turn_for_pending_work().await;
|
||||
@@ -608,8 +611,11 @@ impl Session {
|
||||
}
|
||||
};
|
||||
if let Some(turn_state) = turn_state.as_ref() {
|
||||
let mut ts = turn_state.lock().await;
|
||||
pending_input = ts.take_pending_input();
|
||||
pending_input = self
|
||||
.input_queue
|
||||
.take_pending_input_for_turn_state(turn_state.as_ref())
|
||||
.await;
|
||||
let ts = turn_state.lock().await;
|
||||
turn_had_memory_citation = ts.has_memory_citation;
|
||||
turn_tool_calls = ts.tool_calls;
|
||||
token_usage_at_turn_start = Some(ts.token_usage_at_turn_start.clone());
|
||||
@@ -756,7 +762,8 @@ impl Session {
|
||||
.time_to_first_token_ms()
|
||||
.await;
|
||||
if should_clear_active_turn {
|
||||
self.emit_turn_stop_lifecycle(turn_context.extension_data.as_ref());
|
||||
self.emit_turn_stop_lifecycle(turn_context.extension_data.as_ref())
|
||||
.await;
|
||||
}
|
||||
if let Err(err) = self
|
||||
.goal_runtime_apply(GoalRuntimeEvent::TurnFinished {
|
||||
|
||||
@@ -80,7 +80,7 @@ impl SessionTask for RegularTask {
|
||||
)
|
||||
.instrument(run_turn_span.clone())
|
||||
.await;
|
||||
if !sess.has_pending_input().await {
|
||||
if !sess.input_queue.has_pending_input(&sess.active_turn).await {
|
||||
return last_agent_message;
|
||||
}
|
||||
next_input = Vec::new();
|
||||
|
||||
@@ -1228,7 +1228,7 @@ impl ThreadManagerState {
|
||||
.finalize_thread_spawn(codex, thread_id, tracked_session_source)
|
||||
.await?;
|
||||
if is_resumed_thread {
|
||||
new_thread.thread.emit_thread_resume_lifecycle();
|
||||
new_thread.thread.emit_thread_resume_lifecycle().await;
|
||||
if let Err(err) = new_thread.thread.apply_goal_resume_runtime_effects().await {
|
||||
warn!("failed to apply goal resume runtime effects: {err}");
|
||||
}
|
||||
|
||||
@@ -1561,6 +1561,7 @@ async fn resumed_thread_keeps_paused_goal_paused() -> anyhow::Result<()> {
|
||||
.state_db()
|
||||
.expect("source thread should have a state db");
|
||||
state_db
|
||||
.thread_goals()
|
||||
.replace_thread_goal(
|
||||
source.thread_id,
|
||||
"Keep working until the task is done",
|
||||
@@ -1581,6 +1582,7 @@ async fn resumed_thread_keeps_paused_goal_paused() -> anyhow::Result<()> {
|
||||
.await
|
||||
.expect("resume source thread");
|
||||
let goal = state_db
|
||||
.thread_goals()
|
||||
.get_thread_goal(resumed.thread_id)
|
||||
.await?
|
||||
.expect("goal should still exist after resume");
|
||||
|
||||
@@ -116,7 +116,6 @@ pub(crate) enum ToolEmitter {
|
||||
cwd: AbsolutePathBuf,
|
||||
source: ExecCommandSource,
|
||||
parsed_cmd: Vec<ParsedCommand>,
|
||||
freeform: bool,
|
||||
},
|
||||
ApplyPatch {
|
||||
changes: HashMap<PathBuf, FileChange>,
|
||||
@@ -132,19 +131,13 @@ pub(crate) enum ToolEmitter {
|
||||
}
|
||||
|
||||
impl ToolEmitter {
|
||||
pub fn shell(
|
||||
command: Vec<String>,
|
||||
cwd: AbsolutePathBuf,
|
||||
source: ExecCommandSource,
|
||||
freeform: bool,
|
||||
) -> Self {
|
||||
pub fn shell(command: Vec<String>, cwd: AbsolutePathBuf, source: ExecCommandSource) -> Self {
|
||||
let parsed_cmd = parse_command(&command);
|
||||
Self::Shell {
|
||||
command,
|
||||
cwd,
|
||||
source,
|
||||
parsed_cmd,
|
||||
freeform,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -328,12 +321,7 @@ impl ToolEmitter {
|
||||
output: &ExecToolCallOutput,
|
||||
ctx: ToolEventCtx<'_>,
|
||||
) -> String {
|
||||
match self {
|
||||
Self::Shell { freeform: true, .. } => {
|
||||
super::format_exec_output_for_model_freeform(output, ctx.turn.truncation_policy)
|
||||
}
|
||||
_ => super::format_exec_output_for_model_structured(output, ctx.turn.truncation_policy),
|
||||
}
|
||||
super::format_exec_output_for_model(output, ctx.turn.truncation_policy)
|
||||
}
|
||||
|
||||
pub async fn finish(
|
||||
|
||||
@@ -3,7 +3,6 @@ use codex_apply_patch::MaybeApplyPatchVerified;
|
||||
use codex_exec_server::LOCAL_FS;
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_protocol::protocol::FileChange;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use core_test_support::PathBufExt;
|
||||
use core_test_support::PathExt;
|
||||
use pretty_assertions::assert_eq;
|
||||
@@ -251,12 +250,11 @@ fn write_permissions_for_paths_skip_dirs_already_writable_under_workspace_root()
|
||||
std::fs::create_dir_all(&nested).expect("create nested dir");
|
||||
let file_path = AbsolutePathBuf::try_from(nested.join("file.txt"))
|
||||
.expect("nested file path should be absolute");
|
||||
let sandbox_policy = FileSystemSandboxPolicy::from(&SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![],
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: false,
|
||||
});
|
||||
let sandbox_policy = FileSystemSandboxPolicy::workspace_write(
|
||||
&[],
|
||||
/*exclude_tmpdir_env_var*/ true,
|
||||
/*exclude_slash_tmp*/ false,
|
||||
);
|
||||
|
||||
let permissions = write_permissions_for_paths(&[file_path], &sandbox_policy, &cwd);
|
||||
|
||||
@@ -273,12 +271,11 @@ fn write_permissions_for_paths_keep_dirs_outside_workspace_root() {
|
||||
let file_path = AbsolutePathBuf::try_from(outside.join("file.txt"))
|
||||
.expect("outside file path should be absolute");
|
||||
let cwd_abs = cwd.abs();
|
||||
let sandbox_policy = FileSystemSandboxPolicy::from(&SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![],
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: true,
|
||||
});
|
||||
let sandbox_policy = FileSystemSandboxPolicy::workspace_write(
|
||||
&[],
|
||||
/*exclude_tmpdir_env_var*/ true,
|
||||
/*exclude_slash_tmp*/ true,
|
||||
);
|
||||
|
||||
let permissions = write_permissions_for_paths(&[file_path], &sandbox_policy, &cwd_abs);
|
||||
let expected_outside =
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
//! Built-in model tool handlers for persisted thread goals.
|
||||
//!
|
||||
//! The public tool contract intentionally splits goal creation from completion:
|
||||
//! `create_goal` starts an active objective, while `update_goal` can only mark
|
||||
//! the existing goal complete.
|
||||
//! The public tool contract intentionally splits goal creation from stopped
|
||||
//! status updates: `create_goal` starts an active objective, while
|
||||
//! `update_goal` can only mark the existing goal complete or blocked.
|
||||
|
||||
use crate::function_tool::FunctionCallError;
|
||||
use crate::tools::context::FunctionToolOutput;
|
||||
|
||||
@@ -51,9 +51,12 @@ impl ToolExecutor<ToolInvocation> for UpdateGoalHandler {
|
||||
};
|
||||
|
||||
let args: UpdateGoalArgs = parse_arguments(&arguments)?;
|
||||
if args.status != ThreadGoalStatus::Complete {
|
||||
if !matches!(
|
||||
args.status,
|
||||
ThreadGoalStatus::Complete | ThreadGoalStatus::Blocked
|
||||
) {
|
||||
return Err(FunctionCallError::RespondToModel(
|
||||
"update_goal can only mark the existing goal complete; pause, resume, and budget-limited status changes are controlled by the user or system"
|
||||
"update_goal can only mark the existing goal complete or blocked; pause, resume, budget-limited, and usage-limited status changes are controlled by the user or system"
|
||||
.to_string(),
|
||||
));
|
||||
}
|
||||
@@ -68,13 +71,18 @@ impl ToolExecutor<ToolInvocation> for UpdateGoalHandler {
|
||||
turn.as_ref(),
|
||||
SetGoalRequest {
|
||||
objective: None,
|
||||
status: Some(ThreadGoalStatus::Complete),
|
||||
status: Some(args.status),
|
||||
token_budget: None,
|
||||
},
|
||||
)
|
||||
.await
|
||||
.map_err(|err| FunctionCallError::RespondToModel(format_goal_error(err)))?;
|
||||
goal_response(Some(goal), CompletionBudgetReport::Include).map(boxed_tool_output)
|
||||
let completion_budget_report = if args.status == ThreadGoalStatus::Complete {
|
||||
CompletionBudgetReport::Include
|
||||
} else {
|
||||
CompletionBudgetReport::Omit
|
||||
};
|
||||
goal_response(Some(goal), completion_budget_report).map(boxed_tool_output)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -63,9 +63,9 @@ pub fn create_update_goal_tool() -> ToolSpec {
|
||||
let properties = BTreeMap::from([(
|
||||
"status".to_string(),
|
||||
JsonSchema::string_enum(
|
||||
vec![json!("complete")],
|
||||
vec![json!("complete"), json!("blocked")],
|
||||
Some(
|
||||
"Required. Set to complete only when the objective is achieved and no required work remains."
|
||||
"Required. Set to `complete` only when the objective is achieved and no required work remains. Set to `blocked` only after the same blocking condition has recurred for at least three consecutive goal turns and the agent is at an impasse. After a previously blocked goal is resumed, the resumed run starts a fresh blocked audit."
|
||||
.to_string(),
|
||||
),
|
||||
),
|
||||
@@ -74,10 +74,14 @@ pub fn create_update_goal_tool() -> ToolSpec {
|
||||
ToolSpec::Function(ResponsesApiTool {
|
||||
name: UPDATE_GOAL_TOOL_NAME.to_string(),
|
||||
description: r#"Update the existing goal.
|
||||
Use this tool only to mark the goal achieved.
|
||||
Use this tool only to mark the goal achieved or genuinely blocked.
|
||||
Set status to `complete` only when the objective has actually been achieved and no required work remains.
|
||||
Set status to `blocked` only when the same blocking condition has repeated for at least three consecutive goal turns, counting the original/user-triggered turn and any automatic continuations, and the agent cannot make meaningful progress without user input or an external-state change.
|
||||
If the user resumes a goal that was previously marked `blocked`, treat the resumed run as a fresh blocked audit. If the same blocking condition then repeats for at least three consecutive resumed goal turns, set status to `blocked` again.
|
||||
Once the blocked threshold is satisfied, do not keep reporting that you are still blocked while leaving the goal active; set status to `blocked`.
|
||||
Do not use `blocked` merely because the work is hard, slow, uncertain, incomplete, or would benefit from clarification.
|
||||
Do not mark a goal complete merely because its budget is nearly exhausted or because you are stopping work.
|
||||
You cannot use this tool to pause, resume, or budget-limit a goal; those status changes are controlled by the user or system.
|
||||
You cannot use this tool to pause, resume, budget-limit, or usage-limit a goal; those status changes are controlled by the user or system.
|
||||
When marking a budgeted goal achieved with status `complete`, report the final token usage from the tool result to the user."#
|
||||
.to_string(),
|
||||
strict: false,
|
||||
@@ -96,7 +100,7 @@ mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn update_goal_tool_only_exposes_complete_status() {
|
||||
fn update_goal_tool_exposes_complete_and_blocked_statuses() {
|
||||
let ToolSpec::Function(tool) = create_update_goal_tool() else {
|
||||
panic!("update_goal should be a function tool");
|
||||
};
|
||||
@@ -107,6 +111,9 @@ mod tests {
|
||||
.and_then(|properties| properties.get("status"))
|
||||
.expect("status property should exist");
|
||||
|
||||
assert_eq!(status.enum_values, Some(vec![json!("complete")]));
|
||||
assert_eq!(
|
||||
status.enum_values,
|
||||
Some(vec![json!("complete"), json!("blocked")])
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2792,13 +2792,16 @@ async fn multi_agent_v2_wait_agent_accepts_timeout_only_argument() {
|
||||
});
|
||||
tokio::task::yield_now().await;
|
||||
|
||||
session.enqueue_mailbox_communication(InterAgentCommunication::new(
|
||||
worker_path,
|
||||
AgentPath::root(),
|
||||
Vec::new(),
|
||||
"hello from worker".to_string(),
|
||||
/*trigger_turn*/ false,
|
||||
));
|
||||
session
|
||||
.input_queue
|
||||
.enqueue_mailbox_communication(InterAgentCommunication::new(
|
||||
worker_path,
|
||||
AgentPath::root(),
|
||||
Vec::new(),
|
||||
"hello from worker".to_string(),
|
||||
/*trigger_turn*/ false,
|
||||
))
|
||||
.await;
|
||||
|
||||
let output = wait_task
|
||||
.await
|
||||
@@ -3277,13 +3280,16 @@ async fn multi_agent_v2_wait_agent_returns_summary_for_mailbox_activity() {
|
||||
});
|
||||
tokio::task::yield_now().await;
|
||||
|
||||
session.enqueue_mailbox_communication(InterAgentCommunication::new(
|
||||
worker_path,
|
||||
AgentPath::root(),
|
||||
Vec::new(),
|
||||
"completed".to_string(),
|
||||
/*trigger_turn*/ false,
|
||||
));
|
||||
session
|
||||
.input_queue
|
||||
.enqueue_mailbox_communication(InterAgentCommunication::new(
|
||||
worker_path,
|
||||
AgentPath::root(),
|
||||
Vec::new(),
|
||||
"completed".to_string(),
|
||||
/*trigger_turn*/ false,
|
||||
))
|
||||
.await;
|
||||
|
||||
let wait_output = wait_task
|
||||
.await
|
||||
@@ -3347,13 +3353,16 @@ async fn multi_agent_v2_wait_agent_returns_for_already_queued_mail() {
|
||||
.agent_path
|
||||
.expect("worker path");
|
||||
|
||||
session.enqueue_mailbox_communication(InterAgentCommunication::new(
|
||||
worker_path,
|
||||
AgentPath::root(),
|
||||
Vec::new(),
|
||||
"already queued".to_string(),
|
||||
/*trigger_turn*/ false,
|
||||
));
|
||||
session
|
||||
.input_queue
|
||||
.enqueue_mailbox_communication(InterAgentCommunication::new(
|
||||
worker_path,
|
||||
AgentPath::root(),
|
||||
Vec::new(),
|
||||
"already queued".to_string(),
|
||||
/*trigger_turn*/ false,
|
||||
))
|
||||
.await;
|
||||
|
||||
let output = timeout(
|
||||
Duration::from_millis(500),
|
||||
@@ -3443,13 +3452,16 @@ async fn multi_agent_v2_wait_agent_wakes_on_any_mailbox_notification() {
|
||||
});
|
||||
tokio::task::yield_now().await;
|
||||
|
||||
session.enqueue_mailbox_communication(InterAgentCommunication::new(
|
||||
worker_b_path,
|
||||
AgentPath::root(),
|
||||
Vec::new(),
|
||||
"from worker b".to_string(),
|
||||
/*trigger_turn*/ false,
|
||||
));
|
||||
session
|
||||
.input_queue
|
||||
.enqueue_mailbox_communication(InterAgentCommunication::new(
|
||||
worker_b_path,
|
||||
AgentPath::root(),
|
||||
Vec::new(),
|
||||
"from worker b".to_string(),
|
||||
/*trigger_turn*/ false,
|
||||
))
|
||||
.await;
|
||||
|
||||
let output = wait_task
|
||||
.await
|
||||
@@ -3528,13 +3540,16 @@ async fn multi_agent_v2_wait_agent_does_not_return_completed_content() {
|
||||
});
|
||||
tokio::task::yield_now().await;
|
||||
|
||||
session.enqueue_mailbox_communication(InterAgentCommunication::new(
|
||||
worker_path,
|
||||
AgentPath::root(),
|
||||
Vec::new(),
|
||||
"sensitive child output".to_string(),
|
||||
/*trigger_turn*/ false,
|
||||
));
|
||||
session
|
||||
.input_queue
|
||||
.enqueue_mailbox_communication(InterAgentCommunication::new(
|
||||
worker_path,
|
||||
AgentPath::root(),
|
||||
Vec::new(),
|
||||
"sensitive child output".to_string(),
|
||||
/*trigger_turn*/ false,
|
||||
))
|
||||
.await;
|
||||
|
||||
let output = wait_task
|
||||
.await
|
||||
|
||||
@@ -60,7 +60,7 @@ impl ToolExecutor<ToolInvocation> for Handler {
|
||||
None => default_timeout_ms,
|
||||
};
|
||||
|
||||
let mut mailbox_seq_rx = session.subscribe_mailbox_seq();
|
||||
let mut mailbox_rx = session.input_queue.subscribe_mailbox().await;
|
||||
|
||||
session
|
||||
.send_event(
|
||||
@@ -76,12 +76,8 @@ impl ToolExecutor<ToolInvocation> for Handler {
|
||||
)
|
||||
.await;
|
||||
|
||||
let timed_out = if session.has_pending_mailbox_items().await {
|
||||
false
|
||||
} else {
|
||||
let deadline = Instant::now() + Duration::from_millis(timeout_ms as u64);
|
||||
!wait_for_mailbox_change(&mut mailbox_seq_rx, deadline).await
|
||||
};
|
||||
let deadline = Instant::now() + Duration::from_millis(timeout_ms as u64);
|
||||
let timed_out = !wait_for_mailbox_change(&mut mailbox_rx, deadline).await;
|
||||
let result = WaitAgentResult::from_timed_out(timed_out);
|
||||
|
||||
session
|
||||
@@ -153,10 +149,10 @@ impl ToolOutput for WaitAgentResult {
|
||||
}
|
||||
|
||||
async fn wait_for_mailbox_change(
|
||||
mailbox_seq_rx: &mut tokio::sync::watch::Receiver<u64>,
|
||||
mailbox_rx: &mut tokio::sync::watch::Receiver<()>,
|
||||
deadline: Instant,
|
||||
) -> bool {
|
||||
match timeout_at(deadline, mailbox_seq_rx.changed()).await {
|
||||
match timeout_at(deadline, mailbox_rx.changed()).await {
|
||||
Ok(Ok(())) => true,
|
||||
Ok(Err(_)) | Err(_) => false,
|
||||
}
|
||||
|
||||
@@ -52,7 +52,6 @@ struct RunExecLikeArgs {
|
||||
turn: Arc<TurnContext>,
|
||||
tracker: crate::tools::context::SharedTurnDiffTracker,
|
||||
call_id: String,
|
||||
freeform: bool,
|
||||
shell_runtime_backend: ShellRuntimeBackend,
|
||||
}
|
||||
|
||||
@@ -68,11 +67,9 @@ async fn run_exec_like(args: RunExecLikeArgs) -> Result<FunctionToolOutput, Func
|
||||
turn,
|
||||
tracker,
|
||||
call_id,
|
||||
freeform,
|
||||
shell_runtime_backend,
|
||||
} = args;
|
||||
|
||||
let mut exec_params = exec_params;
|
||||
let Some(turn_environment) = turn.environments.primary() else {
|
||||
return Err(FunctionCallError::RespondToModel(
|
||||
"shell is unavailable in this session".to_string(),
|
||||
@@ -80,18 +77,7 @@ async fn run_exec_like(args: RunExecLikeArgs) -> Result<FunctionToolOutput, Func
|
||||
};
|
||||
let fs = turn_environment.environment.get_filesystem();
|
||||
|
||||
let dependency_env = session.dependency_env().await;
|
||||
if !dependency_env.is_empty() {
|
||||
exec_params.env.extend(dependency_env.clone());
|
||||
}
|
||||
|
||||
let mut explicit_env_overrides = turn.shell_environment_policy.r#set.clone();
|
||||
for key in dependency_env.keys() {
|
||||
if let Some(value) = exec_params.env.get(key) {
|
||||
explicit_env_overrides.insert(key.clone(), value.clone());
|
||||
}
|
||||
}
|
||||
|
||||
let explicit_env_overrides = turn.shell_environment_policy.r#set.clone();
|
||||
let exec_permission_approvals_enabled =
|
||||
session.features().enabled(Feature::ExecPermissionApprovals);
|
||||
let requested_additional_permissions = additional_permissions.clone();
|
||||
@@ -162,12 +148,7 @@ async fn run_exec_like(args: RunExecLikeArgs) -> Result<FunctionToolOutput, Func
|
||||
}
|
||||
|
||||
let source = ExecCommandSource::Agent;
|
||||
let emitter = ToolEmitter::shell(
|
||||
exec_params.command.clone(),
|
||||
exec_params.cwd.clone(),
|
||||
source,
|
||||
freeform,
|
||||
);
|
||||
let emitter = ToolEmitter::shell(exec_params.command.clone(), exec_params.cwd.clone(), source);
|
||||
let event_ctx = ToolEventCtx::new(
|
||||
session.as_ref(),
|
||||
turn.as_ref(),
|
||||
|
||||
@@ -197,7 +197,6 @@ impl ToolExecutor<ToolInvocation> for ShellCommandHandler {
|
||||
turn,
|
||||
tracker,
|
||||
call_id,
|
||||
freeform: true,
|
||||
shell_runtime_backend: self.shell_runtime_backend(),
|
||||
})
|
||||
.await
|
||||
|
||||
@@ -0,0 +1,98 @@
|
||||
use codex_extension_api::ToolCallOutcome;
|
||||
use codex_extension_api::ToolCallSource as ExtensionToolCallSource;
|
||||
use codex_extension_api::ToolFinishInput;
|
||||
use codex_extension_api::ToolStartInput;
|
||||
use codex_tools::ToolName;
|
||||
|
||||
use crate::session::session::Session;
|
||||
use crate::session::turn_context::TurnContext;
|
||||
use crate::tools::context::ToolCallSource;
|
||||
use crate::tools::context::ToolInvocation;
|
||||
|
||||
pub(crate) async fn notify_tool_start(invocation: &ToolInvocation) {
|
||||
for contributor in invocation
|
||||
.session
|
||||
.services
|
||||
.extensions
|
||||
.tool_lifecycle_contributors()
|
||||
{
|
||||
contributor
|
||||
.on_tool_start(ToolStartInput {
|
||||
session_store: &invocation.session.services.session_extension_data,
|
||||
thread_store: &invocation.session.services.thread_extension_data,
|
||||
turn_store: invocation.turn.extension_data.as_ref(),
|
||||
turn_id: invocation.turn.sub_id.as_str(),
|
||||
call_id: invocation.call_id.as_str(),
|
||||
tool_name: &invocation.tool_name,
|
||||
source: extension_tool_call_source(invocation.source.clone()),
|
||||
})
|
||||
.await;
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) async fn notify_tool_finish(invocation: &ToolInvocation, outcome: ToolCallOutcome) {
|
||||
notify_tool_finish_parts(
|
||||
invocation.session.as_ref(),
|
||||
invocation.turn.as_ref(),
|
||||
invocation.call_id.as_str(),
|
||||
&invocation.tool_name,
|
||||
invocation.source.clone(),
|
||||
outcome,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
|
||||
pub(crate) async fn notify_tool_aborted(
|
||||
session: &Session,
|
||||
turn: &TurnContext,
|
||||
call_id: &str,
|
||||
tool_name: &ToolName,
|
||||
source: ToolCallSource,
|
||||
) {
|
||||
notify_tool_finish_parts(
|
||||
session,
|
||||
turn,
|
||||
call_id,
|
||||
tool_name,
|
||||
source,
|
||||
ToolCallOutcome::Aborted,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
|
||||
async fn notify_tool_finish_parts(
|
||||
session: &Session,
|
||||
turn: &TurnContext,
|
||||
call_id: &str,
|
||||
tool_name: &ToolName,
|
||||
source: ToolCallSource,
|
||||
outcome: ToolCallOutcome,
|
||||
) {
|
||||
for contributor in session.services.extensions.tool_lifecycle_contributors() {
|
||||
contributor
|
||||
.on_tool_finish(ToolFinishInput {
|
||||
session_store: &session.services.session_extension_data,
|
||||
thread_store: &session.services.thread_extension_data,
|
||||
turn_store: turn.extension_data.as_ref(),
|
||||
turn_id: turn.sub_id.as_str(),
|
||||
call_id,
|
||||
tool_name,
|
||||
source: extension_tool_call_source(source.clone()),
|
||||
outcome,
|
||||
})
|
||||
.await;
|
||||
}
|
||||
}
|
||||
|
||||
fn extension_tool_call_source(source: ToolCallSource) -> ExtensionToolCallSource {
|
||||
match source {
|
||||
ToolCallSource::Direct => ExtensionToolCallSource::Direct,
|
||||
ToolCallSource::CodeMode {
|
||||
cell_id,
|
||||
runtime_tool_call_id,
|
||||
} => ExtensionToolCallSource::CodeMode {
|
||||
cell_id,
|
||||
runtime_tool_call_id,
|
||||
},
|
||||
}
|
||||
}
|
||||
@@ -4,6 +4,7 @@ pub(crate) mod events;
|
||||
pub(crate) mod handlers;
|
||||
pub(crate) mod hook_names;
|
||||
pub(crate) mod hosted_spec;
|
||||
pub(crate) mod lifecycle;
|
||||
pub(crate) mod network_approval;
|
||||
pub(crate) mod orchestrator;
|
||||
pub(crate) mod parallel;
|
||||
@@ -23,7 +24,6 @@ use codex_utils_output_truncation::TruncationPolicy;
|
||||
use codex_utils_output_truncation::formatted_truncate_text;
|
||||
use codex_utils_output_truncation::truncate_text;
|
||||
pub use router::ToolRouter;
|
||||
use serde::Serialize;
|
||||
|
||||
// Telemetry preview limits: keep log events smaller than model budgets.
|
||||
pub(crate) const TELEMETRY_PREVIEW_MAX_BYTES: usize = 2 * 1024; // 2 KiB
|
||||
@@ -60,46 +60,7 @@ pub(crate) fn tool_user_shell_type(
|
||||
|
||||
/// Format the combined exec output for sending back to the model.
|
||||
/// Includes exit code and duration metadata; truncates large bodies safely.
|
||||
pub fn format_exec_output_for_model_structured(
|
||||
exec_output: &ExecToolCallOutput,
|
||||
truncation_policy: TruncationPolicy,
|
||||
) -> String {
|
||||
let ExecToolCallOutput {
|
||||
exit_code,
|
||||
duration,
|
||||
..
|
||||
} = exec_output;
|
||||
|
||||
#[derive(Serialize)]
|
||||
struct ExecMetadata {
|
||||
exit_code: i32,
|
||||
duration_seconds: f32,
|
||||
}
|
||||
|
||||
#[derive(Serialize)]
|
||||
struct ExecOutput<'a> {
|
||||
output: &'a str,
|
||||
metadata: ExecMetadata,
|
||||
}
|
||||
|
||||
// round to 1 decimal place
|
||||
let duration_seconds = ((duration.as_secs_f32()) * 10.0).round() / 10.0;
|
||||
|
||||
let formatted_output = format_exec_output_str(exec_output, truncation_policy);
|
||||
|
||||
let payload = ExecOutput {
|
||||
output: &formatted_output,
|
||||
metadata: ExecMetadata {
|
||||
exit_code: *exit_code,
|
||||
duration_seconds,
|
||||
},
|
||||
};
|
||||
|
||||
#[expect(clippy::expect_used)]
|
||||
serde_json::to_string(&payload).expect("serialize ExecOutput")
|
||||
}
|
||||
|
||||
pub fn format_exec_output_for_model_freeform(
|
||||
pub fn format_exec_output_for_model(
|
||||
exec_output: &ExecToolCallOutput,
|
||||
truncation_policy: TruncationPolicy,
|
||||
) -> String {
|
||||
|
||||
@@ -1,7 +1,10 @@
|
||||
use std::sync::Arc;
|
||||
use std::sync::atomic::AtomicBool;
|
||||
use std::sync::atomic::Ordering;
|
||||
use std::time::Instant;
|
||||
|
||||
use tokio::sync::RwLock;
|
||||
use tokio::task::JoinError;
|
||||
use tokio_util::either::Either;
|
||||
use tokio_util::sync::CancellationToken;
|
||||
use tokio_util::task::AbortOnDropHandle;
|
||||
@@ -15,6 +18,7 @@ use crate::session::turn_context::TurnContext;
|
||||
use crate::tools::context::AbortedToolOutput;
|
||||
use crate::tools::context::SharedTurnDiffTracker;
|
||||
use crate::tools::context::ToolPayload;
|
||||
use crate::tools::lifecycle::notify_tool_aborted;
|
||||
use crate::tools::registry::AnyToolResult;
|
||||
use crate::tools::registry::ToolArgumentDiffConsumer;
|
||||
use crate::tools::router::ToolCall;
|
||||
@@ -89,6 +93,12 @@ impl ToolCallRuntime {
|
||||
let lock = Arc::clone(&self.parallel_execution);
|
||||
let invocation_cancellation_token = cancellation_token.clone();
|
||||
let started = Instant::now();
|
||||
let abort_session = Arc::clone(&session);
|
||||
let abort_source = source.clone();
|
||||
let abort_turn = Arc::clone(&turn);
|
||||
let terminal_outcome_reached = Arc::new(AtomicBool::new(false));
|
||||
let dispatch_terminal_outcome_reached = Arc::clone(&terminal_outcome_reached);
|
||||
let dispatch_call = call.clone();
|
||||
|
||||
let dispatch_span = trace_span!(
|
||||
"dispatch_tool_call_with_code_mode_result",
|
||||
@@ -97,47 +107,69 @@ impl ToolCallRuntime {
|
||||
call_id = call.call_id.as_str(),
|
||||
aborted = false,
|
||||
);
|
||||
let abort_dispatch_span = dispatch_span.clone();
|
||||
|
||||
let handle: AbortOnDropHandle<Result<AnyToolResult, FunctionCallError>> =
|
||||
let mut handle: AbortOnDropHandle<Result<AnyToolResult, FunctionCallError>> =
|
||||
AbortOnDropHandle::new(tokio::spawn(async move {
|
||||
tokio::select! {
|
||||
_ = cancellation_token.cancelled() => {
|
||||
let secs = started.elapsed().as_secs_f32().max(0.1);
|
||||
dispatch_span.record("aborted", true);
|
||||
Ok(Self::aborted_response(&call, secs))
|
||||
},
|
||||
res = async {
|
||||
let _guard = if supports_parallel {
|
||||
Either::Left(lock.read().await)
|
||||
} else {
|
||||
Either::Right(lock.write().await)
|
||||
};
|
||||
let _guard = if supports_parallel {
|
||||
Either::Left(lock.read().await)
|
||||
} else {
|
||||
Either::Right(lock.write().await)
|
||||
};
|
||||
|
||||
router
|
||||
.dispatch_tool_call_with_code_mode_result(
|
||||
session,
|
||||
turn,
|
||||
invocation_cancellation_token,
|
||||
tracker,
|
||||
call.clone(),
|
||||
source,
|
||||
)
|
||||
.instrument(dispatch_span.clone())
|
||||
.await
|
||||
} => res,
|
||||
}
|
||||
router
|
||||
.dispatch_tool_call_with_terminal_outcome(
|
||||
session,
|
||||
turn,
|
||||
invocation_cancellation_token,
|
||||
tracker,
|
||||
dispatch_call,
|
||||
source,
|
||||
dispatch_terminal_outcome_reached,
|
||||
)
|
||||
.instrument(dispatch_span.clone())
|
||||
.await
|
||||
}));
|
||||
|
||||
async move {
|
||||
handle.await.map_err(|err| {
|
||||
FunctionCallError::Fatal(format!("tool task failed to receive: {err:?}"))
|
||||
})?
|
||||
tokio::select! {
|
||||
res = &mut handle => res.map_err(Self::tool_task_join_error)?,
|
||||
_ = cancellation_token.cancelled() => {
|
||||
if terminal_outcome_reached.load(Ordering::Acquire) || handle.is_finished() {
|
||||
handle.await.map_err(Self::tool_task_join_error)?
|
||||
} else {
|
||||
let secs = started.elapsed().as_secs_f32().max(0.1);
|
||||
abort_dispatch_span.record("aborted", true);
|
||||
handle.abort();
|
||||
match handle.await {
|
||||
Ok(result) => result,
|
||||
Err(err) if err.is_cancelled() => {
|
||||
let response = Self::aborted_response(&call, secs);
|
||||
notify_tool_aborted(
|
||||
abort_session.as_ref(),
|
||||
abort_turn.as_ref(),
|
||||
call.call_id.as_str(),
|
||||
&call.tool_name,
|
||||
abort_source,
|
||||
)
|
||||
.await;
|
||||
Ok(response)
|
||||
}
|
||||
Err(err) => Err(Self::tool_task_join_error(err)),
|
||||
}
|
||||
}
|
||||
},
|
||||
}
|
||||
}
|
||||
.in_current_span()
|
||||
}
|
||||
}
|
||||
|
||||
impl ToolCallRuntime {
|
||||
fn tool_task_join_error(err: JoinError) -> FunctionCallError {
|
||||
FunctionCallError::Fatal(format!("tool task failed to receive: {err:?}"))
|
||||
}
|
||||
|
||||
fn failure_response(call: ToolCall, err: FunctionCallError) -> ResponseInputItem {
|
||||
let message = err.to_string();
|
||||
match call.payload {
|
||||
@@ -189,3 +221,147 @@ impl ToolCallRuntime {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use std::time::Duration;
|
||||
|
||||
use crate::tools::context::FunctionToolOutput;
|
||||
use crate::tools::context::ToolInvocation;
|
||||
use crate::tools::registry::CoreToolRuntime;
|
||||
use crate::tools::registry::ToolExecutor;
|
||||
use crate::tools::registry::ToolRegistry;
|
||||
use crate::turn_diff_tracker::TurnDiffTracker;
|
||||
use codex_extension_api::ToolCallOutcome;
|
||||
use codex_protocol::models::FunctionCallOutputBody;
|
||||
use codex_protocol::models::FunctionCallOutputPayload;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tokio::sync::Notify;
|
||||
use tokio::sync::oneshot;
|
||||
|
||||
struct ImmediateHandler {
|
||||
tool_name: codex_tools::ToolName,
|
||||
}
|
||||
|
||||
#[async_trait::async_trait]
|
||||
impl ToolExecutor<ToolInvocation> for ImmediateHandler {
|
||||
fn tool_name(&self) -> codex_tools::ToolName {
|
||||
self.tool_name.clone()
|
||||
}
|
||||
|
||||
async fn handle(
|
||||
&self,
|
||||
_invocation: ToolInvocation,
|
||||
) -> Result<Box<dyn crate::tools::context::ToolOutput>, FunctionCallError> {
|
||||
Ok(Box::new(FunctionToolOutput::from_text(
|
||||
"ok".to_string(),
|
||||
Some(true),
|
||||
)))
|
||||
}
|
||||
}
|
||||
|
||||
impl CoreToolRuntime for ImmediateHandler {}
|
||||
|
||||
struct BlockingFinishContributor {
|
||||
records: Arc<std::sync::Mutex<Vec<ToolCallOutcome>>>,
|
||||
finish_started: std::sync::Mutex<Option<oneshot::Sender<()>>>,
|
||||
allow_finish: Arc<Notify>,
|
||||
}
|
||||
|
||||
impl codex_extension_api::ToolLifecycleContributor for BlockingFinishContributor {
|
||||
fn on_tool_finish<'a>(
|
||||
&'a self,
|
||||
input: codex_extension_api::ToolFinishInput<'a>,
|
||||
) -> codex_extension_api::ToolLifecycleFuture<'a> {
|
||||
let records = Arc::clone(&self.records);
|
||||
let allow_finish = Arc::clone(&self.allow_finish);
|
||||
let finish_started = self
|
||||
.finish_started
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.take();
|
||||
let outcome = input.outcome;
|
||||
Box::pin(async move {
|
||||
if let Some(finish_started) = finish_started {
|
||||
let _ = finish_started.send(());
|
||||
}
|
||||
allow_finish.notified().await;
|
||||
records
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.push(outcome);
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn cancellation_after_handler_finishes_preserves_completed_lifecycle()
|
||||
-> anyhow::Result<()> {
|
||||
let (mut session, turn_context) = crate::session::tests::make_session_and_context().await;
|
||||
let records = Arc::new(std::sync::Mutex::new(Vec::new()));
|
||||
let (finish_started_tx, finish_started_rx) = oneshot::channel();
|
||||
let allow_finish = Arc::new(Notify::new());
|
||||
let mut builder =
|
||||
codex_extension_api::ExtensionRegistryBuilder::<crate::config::Config>::new();
|
||||
builder.tool_lifecycle_contributor(Arc::new(BlockingFinishContributor {
|
||||
records: Arc::clone(&records),
|
||||
finish_started: std::sync::Mutex::new(Some(finish_started_tx)),
|
||||
allow_finish: Arc::clone(&allow_finish),
|
||||
}));
|
||||
session.services.extensions = Arc::new(builder.build());
|
||||
|
||||
let session = Arc::new(session);
|
||||
let turn_context = Arc::new(turn_context);
|
||||
let tool_name = codex_tools::ToolName::plain("test_tool");
|
||||
let handler = Arc::new(ImmediateHandler {
|
||||
tool_name: tool_name.clone(),
|
||||
}) as Arc<dyn CoreToolRuntime>;
|
||||
let router = Arc::new(ToolRouter::from_parts(
|
||||
ToolRegistry::from_tools([handler]),
|
||||
Vec::new(),
|
||||
));
|
||||
let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new()));
|
||||
let runtime = ToolCallRuntime::new(router, session, turn_context, tracker);
|
||||
let cancellation_token = CancellationToken::new();
|
||||
let call = ToolCall {
|
||||
tool_name,
|
||||
call_id: "call-1".to_string(),
|
||||
payload: ToolPayload::Function {
|
||||
arguments: "{}".to_string(),
|
||||
},
|
||||
};
|
||||
|
||||
let response_task =
|
||||
tokio::spawn(runtime.handle_tool_call(call, cancellation_token.clone()));
|
||||
tokio::time::timeout(Duration::from_secs(1), finish_started_rx)
|
||||
.await
|
||||
.expect("timed out waiting for lifecycle notification to start")
|
||||
.expect("lifecycle notification should start");
|
||||
cancellation_token.cancel();
|
||||
tokio::time::sleep(Duration::from_millis(10)).await;
|
||||
allow_finish.notify_waiters();
|
||||
|
||||
let response = tokio::time::timeout(Duration::from_secs(1), response_task)
|
||||
.await
|
||||
.expect("timed out waiting for tool response")
|
||||
.expect("tool response task should join")?;
|
||||
let expected_response = ResponseInputItem::FunctionCallOutput {
|
||||
call_id: "call-1".to_string(),
|
||||
output: FunctionCallOutputPayload {
|
||||
body: FunctionCallOutputBody::Text("ok".to_string()),
|
||||
success: Some(true),
|
||||
},
|
||||
};
|
||||
assert_eq!(expected_response, response);
|
||||
|
||||
let actual = records
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.drain(..)
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(vec![ToolCallOutcome::Completed { success: true }], actual);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
use std::collections::HashMap;
|
||||
use std::sync::Arc;
|
||||
use std::sync::atomic::AtomicBool;
|
||||
use std::sync::atomic::Ordering;
|
||||
use std::time::Duration;
|
||||
|
||||
use crate::function_tool::FunctionCallError;
|
||||
@@ -18,9 +20,12 @@ use crate::tools::context::ToolOutput;
|
||||
use crate::tools::context::ToolPayload;
|
||||
use crate::tools::flat_tool_name;
|
||||
use crate::tools::hook_names::HookToolName;
|
||||
use crate::tools::lifecycle::notify_tool_finish;
|
||||
use crate::tools::lifecycle::notify_tool_start;
|
||||
use crate::tools::tool_dispatch_trace::ToolDispatchTrace;
|
||||
use crate::tools::tool_search_entry::ToolSearchInfo;
|
||||
use crate::util::error_or_panic;
|
||||
use codex_extension_api::ToolCallOutcome;
|
||||
use codex_protocol::models::ResponseInputItem;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_tools::ToolName;
|
||||
@@ -281,10 +286,6 @@ impl ToolRegistry {
|
||||
self.tools.get(name).map(Arc::clone)
|
||||
}
|
||||
|
||||
pub(crate) fn tool_exposure(&self, name: &ToolName) -> Option<ToolExposure> {
|
||||
self.tools.get(name).map(|tool| tool.exposure())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
pub(crate) fn has_tool(&self, name: &ToolName) -> bool {
|
||||
self.tool(name).is_some()
|
||||
@@ -302,13 +303,23 @@ impl ToolRegistry {
|
||||
Some(tool.supports_parallel_tool_calls())
|
||||
}
|
||||
|
||||
#[allow(dead_code)]
|
||||
pub(crate) async fn dispatch_any(
|
||||
&self,
|
||||
invocation: ToolInvocation,
|
||||
) -> Result<AnyToolResult, FunctionCallError> {
|
||||
self.dispatch_any_with_terminal_outcome(invocation, /*terminal_outcome_reached*/ None)
|
||||
.await
|
||||
}
|
||||
|
||||
#[expect(
|
||||
clippy::await_holding_invalid_type,
|
||||
reason = "tool dispatch must keep active-turn accounting atomic"
|
||||
)]
|
||||
pub(crate) async fn dispatch_any(
|
||||
pub(crate) async fn dispatch_any_with_terminal_outcome(
|
||||
&self,
|
||||
mut invocation: ToolInvocation,
|
||||
terminal_outcome_reached: Option<Arc<AtomicBool>>,
|
||||
) -> Result<AnyToolResult, FunctionCallError> {
|
||||
let tool_name = invocation.tool_name.clone();
|
||||
let tool_name_flat = flat_tool_name(&tool_name);
|
||||
@@ -393,6 +404,8 @@ impl ToolRegistry {
|
||||
return Err(err);
|
||||
}
|
||||
|
||||
notify_tool_start(&invocation).await;
|
||||
|
||||
if let Some(pre_tool_use_payload) = tool.pre_tool_use_payload(&invocation) {
|
||||
match run_pre_tool_use_hooks(
|
||||
&invocation.session,
|
||||
@@ -406,13 +419,33 @@ impl ToolRegistry {
|
||||
PreToolUseHookResult::Blocked(message) => {
|
||||
let err = FunctionCallError::RespondToModel(message);
|
||||
dispatch_trace.record_failed(&err);
|
||||
if let Some(terminal_outcome_reached) = &terminal_outcome_reached {
|
||||
terminal_outcome_reached.store(true, Ordering::Release);
|
||||
}
|
||||
notify_tool_finish(&invocation, ToolCallOutcome::Blocked).await;
|
||||
return Err(err);
|
||||
}
|
||||
PreToolUseHookResult::Continue {
|
||||
updated_input: Some(updated_input),
|
||||
} => {
|
||||
invocation = tool.with_updated_hook_input(invocation, updated_input)?;
|
||||
}
|
||||
} => match tool.with_updated_hook_input(invocation.clone(), updated_input) {
|
||||
Ok(updated_invocation) => {
|
||||
invocation = updated_invocation;
|
||||
}
|
||||
Err(err) => {
|
||||
dispatch_trace.record_failed(&err);
|
||||
if let Some(terminal_outcome_reached) = &terminal_outcome_reached {
|
||||
terminal_outcome_reached.store(true, Ordering::Release);
|
||||
}
|
||||
notify_tool_finish(
|
||||
&invocation,
|
||||
ToolCallOutcome::Failed {
|
||||
handler_executed: false,
|
||||
},
|
||||
)
|
||||
.await;
|
||||
return Err(err);
|
||||
}
|
||||
},
|
||||
PreToolUseHookResult::Continue {
|
||||
updated_input: None,
|
||||
} => {}
|
||||
@@ -507,6 +540,27 @@ impl ToolRegistry {
|
||||
}
|
||||
}
|
||||
|
||||
let lifecycle_outcome = match &result {
|
||||
Ok(_) => {
|
||||
let guard = response_cell.lock().await;
|
||||
match guard.as_ref() {
|
||||
Some(result) => ToolCallOutcome::Completed {
|
||||
success: result.result.success_for_logging(),
|
||||
},
|
||||
None => ToolCallOutcome::Failed {
|
||||
handler_executed: true,
|
||||
},
|
||||
}
|
||||
}
|
||||
Err(_) => ToolCallOutcome::Failed {
|
||||
handler_executed: true,
|
||||
},
|
||||
};
|
||||
if let Some(terminal_outcome_reached) = &terminal_outcome_reached {
|
||||
terminal_outcome_reached.store(true, Ordering::Release);
|
||||
}
|
||||
notify_tool_finish(&invocation, lifecycle_outcome).await;
|
||||
|
||||
if let Err(err) = invocation
|
||||
.session
|
||||
.goal_runtime_apply(GoalRuntimeEvent::ToolCompleted {
|
||||
|
||||
@@ -23,6 +23,97 @@ impl ToolExecutor<ToolInvocation> for TestHandler {
|
||||
|
||||
impl CoreToolRuntime for TestHandler {}
|
||||
|
||||
#[derive(Clone)]
|
||||
enum LifecycleTestResult {
|
||||
Ok { success: bool },
|
||||
Err,
|
||||
}
|
||||
|
||||
struct LifecycleTestHandler {
|
||||
tool_name: codex_tools::ToolName,
|
||||
result: LifecycleTestResult,
|
||||
}
|
||||
|
||||
#[async_trait::async_trait]
|
||||
impl ToolExecutor<ToolInvocation> for LifecycleTestHandler {
|
||||
fn tool_name(&self) -> codex_tools::ToolName {
|
||||
self.tool_name.clone()
|
||||
}
|
||||
|
||||
async fn handle(
|
||||
&self,
|
||||
_invocation: ToolInvocation,
|
||||
) -> Result<Box<dyn crate::tools::context::ToolOutput>, FunctionCallError> {
|
||||
match self.result.clone() {
|
||||
LifecycleTestResult::Ok { success } => Ok(Box::new(
|
||||
crate::tools::context::FunctionToolOutput::from_text(
|
||||
"ok".to_string(),
|
||||
Some(success),
|
||||
),
|
||||
)),
|
||||
LifecycleTestResult::Err => Err(FunctionCallError::RespondToModel(
|
||||
"handler failed".to_string(),
|
||||
)),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl CoreToolRuntime for LifecycleTestHandler {}
|
||||
|
||||
#[derive(Debug, PartialEq, Eq)]
|
||||
enum RecordedToolLifecycle {
|
||||
Start {
|
||||
call_id: String,
|
||||
tool_name: codex_tools::ToolName,
|
||||
},
|
||||
Finish {
|
||||
call_id: String,
|
||||
tool_name: codex_tools::ToolName,
|
||||
outcome: codex_extension_api::ToolCallOutcome,
|
||||
},
|
||||
}
|
||||
|
||||
struct ToolLifecycleRecorder {
|
||||
records: Arc<std::sync::Mutex<Vec<RecordedToolLifecycle>>>,
|
||||
}
|
||||
|
||||
impl codex_extension_api::ToolLifecycleContributor for ToolLifecycleRecorder {
|
||||
fn on_tool_start<'a>(
|
||||
&'a self,
|
||||
input: codex_extension_api::ToolStartInput<'a>,
|
||||
) -> codex_extension_api::ToolLifecycleFuture<'a> {
|
||||
let records = Arc::clone(&self.records);
|
||||
let record = RecordedToolLifecycle::Start {
|
||||
call_id: input.call_id.to_string(),
|
||||
tool_name: input.tool_name.clone(),
|
||||
};
|
||||
Box::pin(async move {
|
||||
records
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.push(record);
|
||||
})
|
||||
}
|
||||
|
||||
fn on_tool_finish<'a>(
|
||||
&'a self,
|
||||
input: codex_extension_api::ToolFinishInput<'a>,
|
||||
) -> codex_extension_api::ToolLifecycleFuture<'a> {
|
||||
let records = Arc::clone(&self.records);
|
||||
let record = RecordedToolLifecycle::Finish {
|
||||
call_id: input.call_id.to_string(),
|
||||
tool_name: input.tool_name.clone(),
|
||||
outcome: input.outcome,
|
||||
};
|
||||
Box::pin(async move {
|
||||
records
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.push(record);
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn handler_looks_up_namespaced_aliases_explicitly() {
|
||||
let namespace = "mcp__codex_apps__gmail";
|
||||
@@ -61,3 +152,106 @@ fn handler_looks_up_namespaced_aliases_explicitly() {
|
||||
.is_some_and(|handler| Arc::ptr_eq(handler, &namespaced_handler))
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn dispatch_notifies_tool_lifecycle_contributors() -> anyhow::Result<()> {
|
||||
let (mut session, turn) = crate::session::tests::make_session_and_context().await;
|
||||
let records = Arc::new(std::sync::Mutex::new(Vec::new()));
|
||||
let mut builder = codex_extension_api::ExtensionRegistryBuilder::<crate::config::Config>::new();
|
||||
builder.tool_lifecycle_contributor(Arc::new(ToolLifecycleRecorder {
|
||||
records: Arc::clone(&records),
|
||||
}));
|
||||
session.services.extensions = Arc::new(builder.build());
|
||||
|
||||
let ok_tool = codex_tools::ToolName::plain("ok_tool");
|
||||
let failing_tool = codex_tools::ToolName::plain("failing_tool");
|
||||
let ok_handler = Arc::new(LifecycleTestHandler {
|
||||
tool_name: ok_tool.clone(),
|
||||
result: LifecycleTestResult::Ok { success: false },
|
||||
}) as Arc<dyn CoreToolRuntime>;
|
||||
let failing_handler = Arc::new(LifecycleTestHandler {
|
||||
tool_name: failing_tool.clone(),
|
||||
result: LifecycleTestResult::Err,
|
||||
}) as Arc<dyn CoreToolRuntime>;
|
||||
let registry = ToolRegistry::new(HashMap::from([
|
||||
(ok_tool.clone(), ok_handler),
|
||||
(failing_tool.clone(), failing_handler),
|
||||
]));
|
||||
let session = Arc::new(session);
|
||||
let turn = Arc::new(turn);
|
||||
|
||||
registry
|
||||
.dispatch_any(test_invocation(
|
||||
Arc::clone(&session),
|
||||
Arc::clone(&turn),
|
||||
"ok-call",
|
||||
ok_tool.clone(),
|
||||
))
|
||||
.await?;
|
||||
let err = match registry
|
||||
.dispatch_any(test_invocation(
|
||||
Arc::clone(&session),
|
||||
Arc::clone(&turn),
|
||||
"failing-call",
|
||||
failing_tool.clone(),
|
||||
))
|
||||
.await
|
||||
{
|
||||
Ok(_) => panic!("failing handler should return an error"),
|
||||
Err(err) => err,
|
||||
};
|
||||
assert_eq!(err.to_string(), "handler failed");
|
||||
|
||||
let expected = vec![
|
||||
RecordedToolLifecycle::Start {
|
||||
call_id: "ok-call".to_string(),
|
||||
tool_name: ok_tool.clone(),
|
||||
},
|
||||
RecordedToolLifecycle::Finish {
|
||||
call_id: "ok-call".to_string(),
|
||||
tool_name: ok_tool,
|
||||
outcome: codex_extension_api::ToolCallOutcome::Completed { success: false },
|
||||
},
|
||||
RecordedToolLifecycle::Start {
|
||||
call_id: "failing-call".to_string(),
|
||||
tool_name: failing_tool.clone(),
|
||||
},
|
||||
RecordedToolLifecycle::Finish {
|
||||
call_id: "failing-call".to_string(),
|
||||
tool_name: failing_tool,
|
||||
outcome: codex_extension_api::ToolCallOutcome::Failed {
|
||||
handler_executed: true,
|
||||
},
|
||||
},
|
||||
];
|
||||
let actual = records
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.drain(..)
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(expected, actual);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn test_invocation(
|
||||
session: Arc<crate::session::session::Session>,
|
||||
turn: Arc<crate::session::turn_context::TurnContext>,
|
||||
call_id: &str,
|
||||
tool_name: codex_tools::ToolName,
|
||||
) -> ToolInvocation {
|
||||
ToolInvocation {
|
||||
session,
|
||||
turn,
|
||||
cancellation_token: tokio_util::sync::CancellationToken::new(),
|
||||
tracker: Arc::new(tokio::sync::Mutex::new(
|
||||
crate::turn_diff_tracker::TurnDiffTracker::new(),
|
||||
)),
|
||||
call_id: call_id.to_string(),
|
||||
tool_name,
|
||||
source: crate::tools::context::ToolCallSource::Direct,
|
||||
payload: ToolPayload::Function {
|
||||
arguments: "{}".to_string(),
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
@@ -19,6 +19,7 @@ use codex_tools::ToolName;
|
||||
use codex_tools::ToolSpec;
|
||||
use codex_tools::ToolsConfig;
|
||||
use std::sync::Arc;
|
||||
use std::sync::atomic::AtomicBool;
|
||||
use tokio_util::sync::CancellationToken;
|
||||
use tracing::instrument;
|
||||
|
||||
@@ -123,6 +124,7 @@ impl ToolRouter {
|
||||
}
|
||||
}
|
||||
|
||||
#[allow(dead_code)]
|
||||
#[instrument(level = "trace", skip_all, err)]
|
||||
pub async fn dispatch_tool_call_with_code_mode_result(
|
||||
&self,
|
||||
@@ -132,6 +134,53 @@ impl ToolRouter {
|
||||
tracker: SharedTurnDiffTracker,
|
||||
call: ToolCall,
|
||||
source: ToolCallSource,
|
||||
) -> Result<AnyToolResult, FunctionCallError> {
|
||||
self.dispatch_tool_call_with_code_mode_result_inner(
|
||||
session,
|
||||
turn,
|
||||
cancellation_token,
|
||||
tracker,
|
||||
call,
|
||||
source,
|
||||
/*terminal_outcome_reached*/ None,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
#[instrument(level = "trace", skip_all, err)]
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub(crate) async fn dispatch_tool_call_with_terminal_outcome(
|
||||
&self,
|
||||
session: Arc<Session>,
|
||||
turn: Arc<TurnContext>,
|
||||
cancellation_token: CancellationToken,
|
||||
tracker: SharedTurnDiffTracker,
|
||||
call: ToolCall,
|
||||
source: ToolCallSource,
|
||||
terminal_outcome_reached: Arc<AtomicBool>,
|
||||
) -> Result<AnyToolResult, FunctionCallError> {
|
||||
self.dispatch_tool_call_with_code_mode_result_inner(
|
||||
session,
|
||||
turn,
|
||||
cancellation_token,
|
||||
tracker,
|
||||
call,
|
||||
source,
|
||||
Some(terminal_outcome_reached),
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
async fn dispatch_tool_call_with_code_mode_result_inner(
|
||||
&self,
|
||||
session: Arc<Session>,
|
||||
turn: Arc<TurnContext>,
|
||||
cancellation_token: CancellationToken,
|
||||
tracker: SharedTurnDiffTracker,
|
||||
call: ToolCall,
|
||||
source: ToolCallSource,
|
||||
terminal_outcome_reached: Option<Arc<AtomicBool>>,
|
||||
) -> Result<AnyToolResult, FunctionCallError> {
|
||||
let ToolCall {
|
||||
tool_name,
|
||||
@@ -150,7 +199,9 @@ impl ToolRouter {
|
||||
payload,
|
||||
};
|
||||
|
||||
self.registry.dispatch_any(invocation).await
|
||||
self.registry
|
||||
.dispatch_any_with_terminal_outcome(invocation, terminal_outcome_reached)
|
||||
.await
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -7,7 +7,6 @@ use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use codex_protocol::protocol::GranularApprovalConfig;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_sandboxing::SandboxManager;
|
||||
use codex_sandboxing::SandboxType;
|
||||
use codex_sandboxing::policy_transforms::effective_file_system_sandbox_policy;
|
||||
@@ -201,8 +200,7 @@ async fn file_system_sandbox_context_uses_active_attempt() {
|
||||
additional_permissions: Some(additional_permissions.clone()),
|
||||
permissions_preapproved: false,
|
||||
};
|
||||
let sandbox_policy = SandboxPolicy::new_read_only_policy();
|
||||
let file_system_policy = FileSystemSandboxPolicy::from(&sandbox_policy);
|
||||
let file_system_policy = FileSystemSandboxPolicy::default();
|
||||
let permissions = PermissionProfile::from_runtime_permissions(
|
||||
&file_system_policy,
|
||||
NetworkSandboxPolicy::Restricted,
|
||||
|
||||
@@ -19,8 +19,6 @@ use codex_protocol::permissions::FileSystemSandboxKind;
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::ReviewDecision;
|
||||
#[cfg(test)]
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_sandboxing::SandboxCommand;
|
||||
use codex_sandboxing::SandboxManager;
|
||||
use codex_sandboxing::SandboxTransformError;
|
||||
|
||||
@@ -5,7 +5,6 @@ use codex_protocol::permissions::FileSystemAccessMode;
|
||||
use codex_protocol::permissions::FileSystemPath;
|
||||
use codex_protocol::permissions::FileSystemSandboxEntry;
|
||||
use codex_protocol::protocol::GranularApprovalConfig;
|
||||
use codex_protocol::protocol::NetworkAccess;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
|
||||
@@ -39,13 +38,10 @@ fn bash_permission_request_payload_includes_description_when_present() {
|
||||
|
||||
#[test]
|
||||
fn external_sandbox_skips_exec_approval_on_request() {
|
||||
let sandbox_policy = SandboxPolicy::ExternalSandbox {
|
||||
network_access: NetworkAccess::Restricted,
|
||||
};
|
||||
assert_eq!(
|
||||
default_exec_approval_requirement(
|
||||
AskForApproval::OnRequest,
|
||||
&FileSystemSandboxPolicy::from(&sandbox_policy),
|
||||
&FileSystemSandboxPolicy::external_sandbox(),
|
||||
),
|
||||
ExecApprovalRequirement::Skip {
|
||||
bypass_sandbox: false,
|
||||
@@ -56,11 +52,10 @@ fn external_sandbox_skips_exec_approval_on_request() {
|
||||
|
||||
#[test]
|
||||
fn restricted_sandbox_requires_exec_approval_on_request() {
|
||||
let sandbox_policy = SandboxPolicy::new_read_only_policy();
|
||||
assert_eq!(
|
||||
default_exec_approval_requirement(
|
||||
AskForApproval::OnRequest,
|
||||
&FileSystemSandboxPolicy::from(&sandbox_policy)
|
||||
&FileSystemSandboxPolicy::default()
|
||||
),
|
||||
ExecApprovalRequirement::NeedsApproval {
|
||||
reason: None,
|
||||
@@ -79,9 +74,8 @@ fn default_exec_approval_requirement_rejects_sandbox_prompt_when_granular_disabl
|
||||
mcp_elicitations: true,
|
||||
});
|
||||
|
||||
let sandbox_policy = SandboxPolicy::new_read_only_policy();
|
||||
let requirement =
|
||||
default_exec_approval_requirement(policy, &FileSystemSandboxPolicy::from(&sandbox_policy));
|
||||
default_exec_approval_requirement(policy, &FileSystemSandboxPolicy::default());
|
||||
|
||||
assert_eq!(
|
||||
requirement,
|
||||
@@ -101,9 +95,8 @@ fn default_exec_approval_requirement_keeps_prompt_when_granular_allows_sandbox_a
|
||||
mcp_elicitations: false,
|
||||
});
|
||||
|
||||
let sandbox_policy = SandboxPolicy::new_read_only_policy();
|
||||
let requirement =
|
||||
default_exec_approval_requirement(policy, &FileSystemSandboxPolicy::from(&sandbox_policy));
|
||||
default_exec_approval_requirement(policy, &FileSystemSandboxPolicy::default());
|
||||
|
||||
assert_eq!(
|
||||
requirement,
|
||||
|
||||
@@ -2,6 +2,7 @@ use crate::config::DEFAULT_MULTI_AGENT_V2_DEFAULT_WAIT_TIMEOUT_MS;
|
||||
use crate::config::DEFAULT_MULTI_AGENT_V2_MAX_WAIT_TIMEOUT_MS;
|
||||
use crate::config::DEFAULT_MULTI_AGENT_V2_MIN_WAIT_TIMEOUT_MS;
|
||||
use crate::tools::code_mode::execute_spec::create_code_mode_tool;
|
||||
use crate::tools::context::ToolInvocation;
|
||||
use crate::tools::handlers::ApplyPatchHandler;
|
||||
use crate::tools::handlers::CodeModeExecuteHandler;
|
||||
use crate::tools::handlers::CodeModeWaitHandler;
|
||||
@@ -58,12 +59,14 @@ use codex_mcp::ToolInfo;
|
||||
use codex_protocol::dynamic_tools::DynamicToolSpec;
|
||||
use codex_protocol::openai_models::ConfigShellToolType;
|
||||
use codex_tools::DiscoverableTool;
|
||||
use codex_tools::ResponsesApiNamespace;
|
||||
use codex_tools::ResponsesApiNamespaceTool;
|
||||
use codex_tools::TOOL_SEARCH_TOOL_NAME;
|
||||
use codex_tools::ToolCall as ExtensionToolCall;
|
||||
use codex_tools::ToolEnvironmentMode;
|
||||
use codex_tools::ToolExecutor;
|
||||
use codex_tools::ToolName;
|
||||
use codex_tools::ToolOutput;
|
||||
use codex_tools::ToolSpec;
|
||||
use codex_tools::ToolsConfig;
|
||||
use codex_tools::collect_code_mode_exec_prompt_tool_definitions;
|
||||
@@ -73,6 +76,8 @@ use std::collections::HashSet;
|
||||
use std::sync::Arc;
|
||||
use tracing::warn;
|
||||
|
||||
const MULTI_AGENT_V2_NAMESPACE_DESCRIPTION: &str = "Tools for spawning and managing sub-agents.";
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
struct ToolRegistryBuildParams<'a> {
|
||||
mcp_tools: Option<&'a [ToolInfo]>,
|
||||
@@ -127,22 +132,29 @@ fn build_model_visible_specs_and_registry(
|
||||
let mut specs = Vec::new();
|
||||
let mut seen_tool_names = HashSet::new();
|
||||
for executor in &executors {
|
||||
if !seen_tool_names.insert(executor.tool_name()) {
|
||||
let tool_name = executor.tool_name();
|
||||
if !seen_tool_names.insert(tool_name.clone()) {
|
||||
continue;
|
||||
}
|
||||
if executor.exposure().is_direct()
|
||||
let exposure = executor.exposure();
|
||||
if exposure.is_direct()
|
||||
&& !is_hidden_by_code_mode_only(config, &tool_name, exposure)
|
||||
&& let Some(spec) = executor.spec()
|
||||
{
|
||||
specs.push(spec_for_model_request(config, executor.exposure(), spec));
|
||||
specs.push(spec_for_model_request(config, exposure, spec));
|
||||
}
|
||||
}
|
||||
for spec in hosted_specs {
|
||||
if !is_hidden_by_code_mode_only(config, &ToolName::plain(spec.name()), ToolExposure::Direct)
|
||||
{
|
||||
specs.push(spec);
|
||||
}
|
||||
}
|
||||
specs.extend(hosted_specs);
|
||||
|
||||
let registry = ToolRegistry::from_tools(executors);
|
||||
let model_visible_specs = merge_into_namespaces(specs)
|
||||
.into_iter()
|
||||
.filter(|spec| config.namespace_tools || !matches!(spec, ToolSpec::Namespace(_)))
|
||||
.filter(|spec| !is_hidden_by_code_mode_only(config, ®istry, spec))
|
||||
.collect();
|
||||
|
||||
(model_visible_specs, registry)
|
||||
@@ -210,17 +222,14 @@ fn agent_type_description(config: &ToolsConfig, default_agent_type_description:
|
||||
|
||||
fn is_hidden_by_code_mode_only(
|
||||
config: &ToolsConfig,
|
||||
registry: &ToolRegistry,
|
||||
spec: &ToolSpec,
|
||||
tool_name: &ToolName,
|
||||
exposure: ToolExposure,
|
||||
) -> bool {
|
||||
if !config.code_mode_only_enabled || !codex_code_mode::is_code_mode_nested_tool(spec.name()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
let exposure = registry
|
||||
.tool_exposure(&ToolName::plain(spec.name()))
|
||||
.unwrap_or(ToolExposure::Direct);
|
||||
exposure != ToolExposure::DirectModelOnly
|
||||
config.code_mode_only_enabled
|
||||
&& exposure != ToolExposure::DirectModelOnly
|
||||
&& codex_code_mode::is_code_mode_nested_tool(&codex_tools::code_mode_name_for_tool_name(
|
||||
tool_name,
|
||||
))
|
||||
}
|
||||
|
||||
fn build_code_mode_executors(
|
||||
@@ -442,6 +451,10 @@ fn collect_tool_executors(
|
||||
} else {
|
||||
ToolExposure::Direct
|
||||
};
|
||||
let tool_namespace = config
|
||||
.namespace_tools
|
||||
.then_some(config.multi_agent_v2_tool_namespace.as_deref())
|
||||
.flatten();
|
||||
let agent_type_description =
|
||||
agent_type_description(config, params.default_agent_type_description);
|
||||
executors.push(multi_agent_v2_handler(
|
||||
@@ -454,15 +467,33 @@ fn collect_tool_executors(
|
||||
max_concurrent_threads_per_session: config.max_concurrent_threads_per_session,
|
||||
}),
|
||||
exposure,
|
||||
tool_namespace,
|
||||
));
|
||||
executors.push(multi_agent_v2_handler(
|
||||
SendMessageHandlerV2,
|
||||
exposure,
|
||||
tool_namespace,
|
||||
));
|
||||
executors.push(multi_agent_v2_handler(
|
||||
FollowupTaskHandlerV2,
|
||||
exposure,
|
||||
tool_namespace,
|
||||
));
|
||||
executors.push(multi_agent_v2_handler(SendMessageHandlerV2, exposure));
|
||||
executors.push(multi_agent_v2_handler(FollowupTaskHandlerV2, exposure));
|
||||
executors.push(multi_agent_v2_handler(
|
||||
WaitAgentHandlerV2::new(params.wait_agent_timeouts),
|
||||
exposure,
|
||||
tool_namespace,
|
||||
));
|
||||
executors.push(multi_agent_v2_handler(
|
||||
CloseAgentHandlerV2,
|
||||
exposure,
|
||||
tool_namespace,
|
||||
));
|
||||
executors.push(multi_agent_v2_handler(
|
||||
ListAgentsHandlerV2,
|
||||
exposure,
|
||||
tool_namespace,
|
||||
));
|
||||
executors.push(multi_agent_v2_handler(CloseAgentHandlerV2, exposure));
|
||||
executors.push(multi_agent_v2_handler(ListAgentsHandlerV2, exposure));
|
||||
} else {
|
||||
let agent_type_description =
|
||||
agent_type_description(config, params.default_agent_type_description);
|
||||
@@ -592,8 +623,70 @@ fn append_extension_tool_executors(
|
||||
fn multi_agent_v2_handler(
|
||||
handler: impl CoreToolRuntime + 'static,
|
||||
exposure: ToolExposure,
|
||||
namespace: Option<&str>,
|
||||
) -> Arc<dyn CoreToolRuntime> {
|
||||
override_tool_exposure(Arc::new(handler), exposure)
|
||||
let handler: Arc<dyn CoreToolRuntime> = match namespace {
|
||||
Some(namespace) => Arc::new(MultiAgentV2NamespaceOverride {
|
||||
handler: Arc::new(handler),
|
||||
namespace: namespace.to_string(),
|
||||
}),
|
||||
None => Arc::new(handler),
|
||||
};
|
||||
override_tool_exposure(handler, exposure)
|
||||
}
|
||||
|
||||
struct MultiAgentV2NamespaceOverride {
|
||||
handler: Arc<dyn CoreToolRuntime>,
|
||||
namespace: String,
|
||||
}
|
||||
|
||||
#[async_trait::async_trait]
|
||||
impl ToolExecutor<ToolInvocation> for MultiAgentV2NamespaceOverride {
|
||||
fn tool_name(&self) -> ToolName {
|
||||
ToolName::namespaced(self.namespace.clone(), self.handler.tool_name().name)
|
||||
}
|
||||
|
||||
fn spec(&self) -> Option<ToolSpec> {
|
||||
match self.handler.spec()? {
|
||||
ToolSpec::Function(tool) => Some(ToolSpec::Namespace(ResponsesApiNamespace {
|
||||
name: self.namespace.clone(),
|
||||
description: MULTI_AGENT_V2_NAMESPACE_DESCRIPTION.to_string(),
|
||||
tools: vec![ResponsesApiNamespaceTool::Function(tool)],
|
||||
})),
|
||||
spec => Some(spec),
|
||||
}
|
||||
}
|
||||
|
||||
fn exposure(&self) -> ToolExposure {
|
||||
self.handler.exposure()
|
||||
}
|
||||
|
||||
fn supports_parallel_tool_calls(&self) -> bool {
|
||||
self.handler.supports_parallel_tool_calls()
|
||||
}
|
||||
|
||||
async fn handle(
|
||||
&self,
|
||||
invocation: ToolInvocation,
|
||||
) -> Result<Box<dyn ToolOutput>, codex_tools::FunctionCallError> {
|
||||
self.handler.handle(invocation).await
|
||||
}
|
||||
}
|
||||
|
||||
impl CoreToolRuntime for MultiAgentV2NamespaceOverride {
|
||||
fn matches_kind(&self, payload: &crate::tools::context::ToolPayload) -> bool {
|
||||
self.handler.matches_kind(payload)
|
||||
}
|
||||
|
||||
fn search_info(&self) -> Option<crate::tools::tool_search_entry::ToolSearchInfo> {
|
||||
self.handler.search_info()
|
||||
}
|
||||
|
||||
fn create_diff_consumer(
|
||||
&self,
|
||||
) -> Option<Box<dyn crate::tools::registry::ToolArgumentDiffConsumer>> {
|
||||
self.handler.create_diff_consumer()
|
||||
}
|
||||
}
|
||||
|
||||
fn compare_code_mode_tools(
|
||||
|
||||
@@ -793,7 +793,6 @@ async fn request_plugin_install_requires_apps_and_plugins_features() {
|
||||
|
||||
for disabled_feature in [Feature::Apps, Feature::Plugins] {
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::ToolSearch);
|
||||
features.enable(Feature::ToolSuggest);
|
||||
features.enable(Feature::Apps);
|
||||
features.enable(Feature::Plugins);
|
||||
@@ -832,7 +831,6 @@ async fn search_tool_is_hidden_without_deferred_tools() {
|
||||
let model_info = search_capable_model_info().await;
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::Apps);
|
||||
features.enable(Feature::ToolSearch);
|
||||
let available_models = Vec::new();
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
@@ -863,7 +861,6 @@ async fn search_tool_description_falls_back_to_connector_name_without_descriptio
|
||||
let model_info = search_capable_model_info().await;
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::Apps);
|
||||
features.enable(Feature::ToolSearch);
|
||||
let available_models = Vec::new();
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
@@ -907,7 +904,7 @@ async fn search_tool_description_falls_back_to_connector_name_without_descriptio
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_mcp_tool_property_missing_type_defaults_to_string() {
|
||||
async fn test_mcp_tool_property_missing_type_defaults_to_empty_schema() {
|
||||
let config = test_config().await;
|
||||
let model_info = construct_model_info_offline("gpt-5.4", &config);
|
||||
let mut features = Features::with_defaults();
|
||||
@@ -950,10 +947,7 @@ async fn test_mcp_tool_property_missing_type_defaults_to_string() {
|
||||
name: "search".to_string(),
|
||||
parameters: JsonSchema::object(
|
||||
/*properties*/
|
||||
BTreeMap::from([(
|
||||
"query".to_string(),
|
||||
JsonSchema::string(Some("search query".to_string())),
|
||||
)]),
|
||||
BTreeMap::from([("query".to_string(), JsonSchema::default())]),
|
||||
/*required*/ None,
|
||||
/*additional_properties*/ None
|
||||
),
|
||||
@@ -1341,3 +1335,67 @@ async fn code_mode_only_can_expose_multi_agent_v2_as_normal_tools() {
|
||||
};
|
||||
assert!(!spawn_agent.description.contains("exec tool declaration"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn code_mode_only_can_expose_namespaced_multi_agent_v2_as_normal_tools() {
|
||||
let config = test_config().await;
|
||||
let model_info = construct_model_info_offline("gpt-5.4", &config);
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::CodeMode);
|
||||
features.enable(Feature::CodeModeOnly);
|
||||
features.enable(Feature::MultiAgentV2);
|
||||
let available_models = Vec::new();
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
available_models: &available_models,
|
||||
features: &features,
|
||||
image_generation_tool_auth_allowed: true,
|
||||
web_search_mode: Some(WebSearchMode::Live),
|
||||
session_source: SessionSource::Cli,
|
||||
permission_profile: &PermissionProfile::Disabled,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
})
|
||||
.with_multi_agent_v2_tool_namespace(Some("agents".to_string()))
|
||||
.with_multi_agent_v2_non_code_mode_only(/*multi_agent_v2_non_code_mode_only*/ true);
|
||||
let router = ToolRouter::from_config(
|
||||
&tools_config,
|
||||
ToolRouterParams {
|
||||
mcp_tools: None,
|
||||
deferred_mcp_tools: None,
|
||||
discoverable_tools: None,
|
||||
extension_tool_executors: Vec::new(),
|
||||
dynamic_tools: &[],
|
||||
},
|
||||
);
|
||||
let model_visible_specs = router.model_visible_specs();
|
||||
let tool_names = model_visible_specs
|
||||
.iter()
|
||||
.map(ToolSpec::name)
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
assert_eq!(tool_names, vec!["exec", "wait", "agents"]);
|
||||
|
||||
let exec = find_tool(&model_visible_specs, "exec");
|
||||
let ToolSpec::Freeform(exec) = exec else {
|
||||
panic!("exec should be a freeform tool");
|
||||
};
|
||||
assert!(!exec.description.contains("spawn_agent"));
|
||||
assert!(!exec.description.contains("wait_agent"));
|
||||
assert!(
|
||||
!exec
|
||||
.description
|
||||
.contains("do not attempt to use any other tools directly")
|
||||
);
|
||||
|
||||
for tool_name in [
|
||||
"spawn_agent",
|
||||
"send_message",
|
||||
"followup_task",
|
||||
"wait_agent",
|
||||
"close_agent",
|
||||
"list_agents",
|
||||
] {
|
||||
let tool = find_namespace_function_tool(&model_visible_specs, "agents", tool_name);
|
||||
assert!(!tool.description.contains("exec tool declaration"));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -577,6 +577,77 @@ fn test_build_specs_multi_agent_v2_uses_task_names_and_hides_resume() {
|
||||
assert_lacks_tool_name(&tools, "resume_agent");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_build_specs_multi_agent_v2_uses_configured_tool_namespace() {
|
||||
let model_info = model_info();
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::MultiAgentV2);
|
||||
let available_models = Vec::new();
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
available_models: &available_models,
|
||||
features: &features,
|
||||
image_generation_tool_auth_allowed: true,
|
||||
web_search_mode: Some(WebSearchMode::Cached),
|
||||
session_source: SessionSource::Cli,
|
||||
permission_profile: &PermissionProfile::Disabled,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
})
|
||||
.with_multi_agent_v2_tool_namespace(Some("agents".to_string()));
|
||||
let (tools, registry) = build_specs(
|
||||
&tools_config,
|
||||
/*mcp_tools*/ None,
|
||||
/*deferred_mcp_tools*/ None,
|
||||
&[],
|
||||
);
|
||||
|
||||
assert_contains_tool_names(&tools, &["agents"]);
|
||||
for tool_name in [
|
||||
"spawn_agent",
|
||||
"send_message",
|
||||
"followup_task",
|
||||
"wait_agent",
|
||||
"close_agent",
|
||||
"list_agents",
|
||||
] {
|
||||
assert_lacks_tool_name(&tools, tool_name);
|
||||
assert!(registry.has_tool(&ToolName::namespaced("agents", tool_name)));
|
||||
assert!(!registry.has_tool(&ToolName::plain(tool_name)));
|
||||
assert_namespace_contains_function(&tools, "agents", tool_name);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_build_specs_multi_agent_v2_ignores_tool_namespace_without_namespace_support() {
|
||||
let model_info = model_info();
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::MultiAgentV2);
|
||||
let available_models = Vec::new();
|
||||
let mut tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
available_models: &available_models,
|
||||
features: &features,
|
||||
image_generation_tool_auth_allowed: true,
|
||||
web_search_mode: Some(WebSearchMode::Cached),
|
||||
session_source: SessionSource::Cli,
|
||||
permission_profile: &PermissionProfile::Disabled,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
})
|
||||
.with_multi_agent_v2_tool_namespace(Some("agents".to_string()));
|
||||
tools_config.namespace_tools = false;
|
||||
let (tools, registry) = build_specs(
|
||||
&tools_config,
|
||||
/*mcp_tools*/ None,
|
||||
/*deferred_mcp_tools*/ None,
|
||||
&[],
|
||||
);
|
||||
|
||||
assert_contains_tool_names(&tools, &["spawn_agent", "send_message", "list_agents"]);
|
||||
assert_lacks_tool_name(&tools, "agents");
|
||||
assert!(registry.has_tool(&ToolName::plain("spawn_agent")));
|
||||
assert!(!registry.has_tool(&ToolName::namespaced("agents", "spawn_agent")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_build_specs_multi_agent_v2_does_not_require_collab_feature() {
|
||||
let model_info = model_info();
|
||||
@@ -1582,7 +1653,6 @@ fn search_tool_description_lists_each_mcp_source_once() {
|
||||
let model_info = search_capable_model_info();
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::Apps);
|
||||
features.enable(Feature::ToolSearch);
|
||||
let available_models = Vec::new();
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
@@ -1655,7 +1725,7 @@ fn search_tool_description_lists_each_mcp_source_once() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn search_tool_requires_model_capability_and_enabled_feature() {
|
||||
fn search_tool_requires_model_capability() {
|
||||
let model_info = search_capable_model_info();
|
||||
let deferred_mcp_tools = Some(vec![deferred_mcp_tool(
|
||||
"_create_event",
|
||||
@@ -1688,26 +1758,6 @@ fn search_tool_requires_model_capability_and_enabled_feature() {
|
||||
);
|
||||
assert_lacks_tool_name(&tools, TOOL_SEARCH_TOOL_NAME);
|
||||
|
||||
let mut features_without_tool_search = Features::with_defaults();
|
||||
features_without_tool_search.disable(Feature::ToolSearch);
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
available_models: &available_models,
|
||||
features: &features_without_tool_search,
|
||||
image_generation_tool_auth_allowed: true,
|
||||
web_search_mode: Some(WebSearchMode::Cached),
|
||||
session_source: SessionSource::Cli,
|
||||
permission_profile: &PermissionProfile::Disabled,
|
||||
windows_sandbox_level: WindowsSandboxLevel::Disabled,
|
||||
});
|
||||
let (tools, _) = build_specs(
|
||||
&tools_config,
|
||||
/*mcp_tools*/ None,
|
||||
deferred_mcp_tools.clone(),
|
||||
&[],
|
||||
);
|
||||
assert_lacks_tool_name(&tools, TOOL_SEARCH_TOOL_NAME);
|
||||
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
available_models: &available_models,
|
||||
@@ -1730,8 +1780,7 @@ fn search_tool_requires_model_capability_and_enabled_feature() {
|
||||
#[test]
|
||||
fn no_search_tool_when_namespaces_disabled() {
|
||||
let model_info = search_capable_model_info();
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::ToolSearch);
|
||||
let features = Features::with_defaults();
|
||||
let available_models = Vec::new();
|
||||
let mut tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
@@ -1765,8 +1814,7 @@ fn no_search_tool_when_namespaces_disabled() {
|
||||
#[test]
|
||||
fn search_tool_registers_for_deferred_dynamic_tools() {
|
||||
let model_info = search_capable_model_info();
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::ToolSearch);
|
||||
let features = Features::with_defaults();
|
||||
let available_models = Vec::new();
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
@@ -1825,8 +1873,7 @@ fn search_tool_registers_for_deferred_dynamic_tools() {
|
||||
#[test]
|
||||
fn search_tool_is_hidden_for_deferred_dynamic_tools_when_namespace_tools_are_disabled() {
|
||||
let model_info = search_capable_model_info();
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::ToolSearch);
|
||||
let features = Features::with_defaults();
|
||||
let available_models = Vec::new();
|
||||
let mut tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
@@ -1875,7 +1922,6 @@ fn search_tool_is_hidden_for_deferred_dynamic_tools_when_namespace_tools_are_dis
|
||||
fn request_plugin_install_is_not_registered_without_feature_flag() {
|
||||
let model_info = search_capable_model_info();
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::ToolSearch);
|
||||
features.enable(Feature::Apps);
|
||||
features.enable(Feature::Plugins);
|
||||
features.disable(Feature::ToolSuggest);
|
||||
@@ -1965,7 +2011,6 @@ fn request_plugin_install_description_lists_discoverable_tools() {
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::Apps);
|
||||
features.enable(Feature::Plugins);
|
||||
features.enable(Feature::ToolSearch);
|
||||
features.enable(Feature::ToolSuggest);
|
||||
let available_models = Vec::new();
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
@@ -2657,6 +2702,23 @@ fn find_tool<'a>(tools: &'a [ToolSpec], expected_name: &str) -> &'a ToolSpec {
|
||||
.unwrap_or_else(|| panic!("expected tool {expected_name}"))
|
||||
}
|
||||
|
||||
fn assert_namespace_contains_function(
|
||||
tools: &[ToolSpec],
|
||||
expected_namespace: &str,
|
||||
expected_name: &str,
|
||||
) {
|
||||
let namespace_tool = find_tool(tools, expected_namespace);
|
||||
let ToolSpec::Namespace(namespace) = namespace_tool else {
|
||||
panic!("expected namespace tool {expected_namespace}");
|
||||
};
|
||||
assert!(
|
||||
namespace.tools.iter().any(|tool| {
|
||||
matches!(tool, ResponsesApiNamespaceTool::Function(tool) if tool.name == expected_name)
|
||||
}),
|
||||
"expected tool {expected_name} in namespace {expected_namespace}"
|
||||
);
|
||||
}
|
||||
|
||||
fn assert_process_tool_environment_id(
|
||||
tools: &[ToolSpec],
|
||||
expected_name: &str,
|
||||
|
||||
@@ -71,16 +71,10 @@ fn exec_server_params_use_env_policy_overlay_contract() {
|
||||
.expect("current dir")
|
||||
.try_into()
|
||||
.expect("absolute path");
|
||||
let sandbox_policy = codex_protocol::protocol::SandboxPolicy::DangerFullAccess;
|
||||
let file_system_sandbox_policy =
|
||||
codex_protocol::permissions::FileSystemSandboxPolicy::from(&sandbox_policy);
|
||||
codex_protocol::permissions::FileSystemSandboxPolicy::unrestricted();
|
||||
let network_sandbox_policy = codex_protocol::permissions::NetworkSandboxPolicy::Restricted;
|
||||
let permission_profile =
|
||||
codex_protocol::models::PermissionProfile::from_runtime_permissions_with_enforcement(
|
||||
codex_protocol::models::SandboxEnforcement::from_legacy_sandbox_policy(&sandbox_policy),
|
||||
&file_system_sandbox_policy,
|
||||
network_sandbox_policy,
|
||||
);
|
||||
let permission_profile = codex_protocol::models::PermissionProfile::Disabled;
|
||||
let request = ExecRequest {
|
||||
command: vec!["bash".to_string(), "-lc".to_string(), "true".to_string()],
|
||||
cwd: cwd.clone(),
|
||||
|
||||
Reference in New Issue
Block a user