Merge cf1368b80d into sapling-pr-archive-bolinfest

This commit is contained in:
Michael Bolin
2026-03-27 09:13:06 -07:00
committed by GitHub
42 changed files with 728 additions and 382 deletions
+3
View File
@@ -2224,6 +2224,7 @@ dependencies = [
"codex-keyring-store",
"codex-protocol",
"codex-terminal-detection",
"codex-utils-template",
"core_test_support",
"keyring",
"once_cell",
@@ -2385,6 +2386,7 @@ dependencies = [
"codex-utils-absolute-path",
"codex-utils-image",
"codex-utils-string",
"codex-utils-template",
"icu_decimal",
"icu_locale_core",
"icu_provider",
@@ -2612,6 +2614,7 @@ dependencies = [
name = "codex-tools"
version = "0.0.0"
dependencies = [
"codex-protocol",
"pretty_assertions",
"rmcp",
"serde",
+34 -99
View File
@@ -5,10 +5,8 @@ use crate::agent::role::DEFAULT_ROLE_NAME;
use crate::agent::role::resolve_role_config;
use crate::agent::status::is_final;
use crate::codex_thread::ThreadConfigSnapshot;
use crate::context_manager::is_user_turn_boundary;
use crate::error::CodexErr;
use crate::error::Result as CodexResult;
use crate::event_mapping::parse_turn_item;
use crate::find_archived_thread_path_by_id_str;
use crate::find_thread_path_by_id_str;
use crate::rollout::RolloutRecorder;
@@ -20,10 +18,7 @@ use crate::thread_manager::ThreadManagerState;
use codex_features::Feature;
use codex_protocol::AgentPath;
use codex_protocol::ThreadId;
use codex_protocol::items::TurnItem;
use codex_protocol::models::ContentItem;
use codex_protocol::models::FunctionCallOutputPayload;
use codex_protocol::models::ResponseInputItem;
use codex_protocol::models::ResponseItem;
use codex_protocol::protocol::InitialHistory;
use codex_protocol::protocol::InterAgentCommunication;
@@ -118,30 +113,36 @@ impl AgentControl {
pub(crate) async fn spawn_agent(
&self,
config: crate::config::Config,
items: Vec<UserInput>,
initial_operation: Op,
session_source: Option<SessionSource>,
) -> CodexResult<ThreadId> {
Ok(self
.spawn_agent_internal(config, items, session_source, SpawnAgentOptions::default())
.spawn_agent_internal(
config,
initial_operation,
session_source,
SpawnAgentOptions::default(),
)
.await?
.thread_id)
}
/// Spawn an agent thread with some metadata.
pub(crate) async fn spawn_agent_with_metadata(
&self,
config: crate::config::Config,
items: Vec<UserInput>,
initial_operation: Op,
session_source: Option<SessionSource>,
options: SpawnAgentOptions,
options: SpawnAgentOptions, // TODO(jif) drop with new fork.
) -> CodexResult<LiveAgent> {
self.spawn_agent_internal(config, items, session_source, options)
self.spawn_agent_internal(config, initial_operation, session_source, options)
.await
}
async fn spawn_agent_internal(
&self,
config: crate::config::Config,
items: Vec<UserInput>,
initial_operation: Op,
session_source: Option<SessionSource>,
options: SpawnAgentOptions,
) -> CodexResult<LiveAgent> {
@@ -270,7 +271,8 @@ impl AgentControl {
)
.await;
self.send_input(new_thread.thread_id, items).await?;
self.send_input(new_thread.thread_id, initial_operation)
.await?;
let child_reference = agent_metadata
.agent_path
.as_ref()
@@ -468,23 +470,15 @@ impl AgentControl {
pub(crate) async fn send_input(
&self,
agent_id: ThreadId,
items: Vec<UserInput>,
initial_operation: Op,
) -> CodexResult<String> {
let last_task_message = render_input_preview(&items);
let last_task_message = render_input_preview(&initial_operation);
let state = self.upgrade()?;
let result = self
.handle_thread_request_result(
agent_id,
&state,
state
.send_op(
agent_id,
Op::UserInput {
items,
final_output_json_schema: None,
},
)
.await,
state.send_op(agent_id, initial_operation).await,
)
.await;
if result.is_ok() {
@@ -766,10 +760,7 @@ impl AgentControl {
.as_ref()
.map(ToString::to_string)
.unwrap_or_else(|| thread_id.to_string());
let last_task_message = match metadata.last_task_message.clone() {
Some(last_task_message) => Some(last_task_message),
None => last_task_message_for_thread(thread.as_ref()).await,
};
let last_task_message = metadata.last_task_message.clone();
agents.push(ListedAgent {
agent_name,
agent_status: thread.agent_status().await,
@@ -1072,79 +1063,23 @@ fn agent_matches_prefix(agent_path: Option<&AgentPath>, prefix: &AgentPath) -> b
})
}
async fn last_task_message_for_thread(thread: &crate::CodexThread) -> Option<String> {
let pending_input = thread.codex.session.pending_input_snapshot().await;
if let Some(message) = pending_input
.iter()
.rev()
.find_map(last_task_message_from_input_item)
{
return Some(message);
pub(crate) fn render_input_preview(initial_operation: &Op) -> String {
match initial_operation {
Op::UserInput { items, .. } => items
.iter()
.map(|item| match item {
UserInput::Text { text, .. } => text.clone(),
UserInput::Image { .. } => "[image]".to_string(),
UserInput::LocalImage { path } => format!("[local_image:{}]", path.display()),
UserInput::Skill { name, path } => format!("[skill:${name}]({})", path.display()),
UserInput::Mention { name, path } => format!("[mention:${name}]({path})"),
_ => "[input]".to_string(),
})
.collect::<Vec<_>>()
.join("\n"),
Op::InterAgentCommunication { communication } => communication.content.clone(),
_ => String::new(),
}
let queued_input = thread
.codex
.session
.queued_response_items_for_next_turn_snapshot()
.await;
if let Some(message) = queued_input
.iter()
.rev()
.find_map(last_task_message_from_input_item)
{
return Some(message);
}
let history = thread.codex.session.clone_history().await;
history
.raw_items()
.iter()
.rev()
.find_map(last_task_message_from_item)
}
fn last_task_message_from_input_item(item: &ResponseInputItem) -> Option<String> {
let response_item: ResponseItem = item.clone().into();
last_task_message_from_item(&response_item)
}
fn last_task_message_from_item(item: &ResponseItem) -> Option<String> {
if !is_user_turn_boundary(item) {
return None;
}
match item {
ResponseItem::Message { role, .. } if role == "user" => {
let Some(TurnItem::UserMessage(message)) = parse_turn_item(item) else {
return None;
};
Some(render_input_preview(&message.content))
}
ResponseItem::Message { content, .. } => match content.as_slice() {
[ContentItem::InputText { text }] | [ContentItem::OutputText { text }] => {
serde_json::from_str::<InterAgentCommunication>(text)
.ok()
.map(|communication| communication.content)
}
_ => None,
},
_ => None,
}
}
fn render_input_preview(items: &[UserInput]) -> String {
items
.iter()
.map(|item| match item {
UserInput::Text { text, .. } => text.clone(),
UserInput::Image { .. } => "[image]".to_string(),
UserInput::LocalImage { path } => format!("[local_image:{}]", path.display()),
UserInput::Skill { name, path } => format!("[skill:${name}]({})", path.display()),
UserInput::Mention { name, path } => format!("[mention:${name}]({path})"),
_ => "[input]".to_string(),
})
.collect::<Vec<_>>()
.join("\n")
}
fn thread_spawn_depth(session_source: &SessionSource) -> Option<i32> {
+8 -4
View File
@@ -54,11 +54,12 @@ async fn test_config() -> (TempDir, Config) {
test_config_with_cli_overrides(Vec::new()).await
}
fn text_input(text: &str) -> Vec<UserInput> {
fn text_input(text: &str) -> Op {
vec![UserInput::Text {
text: text.to_string(),
text_elements: Vec::new(),
}]
.into()
}
struct AgentControlHarness {
@@ -217,7 +218,8 @@ async fn send_input_errors_when_manager_dropped() {
vec![UserInput::Text {
text: "hello".to_string(),
text_elements: Vec::new(),
}],
}]
.into(),
)
.await
.expect_err("send_input should fail without a manager");
@@ -321,7 +323,8 @@ async fn send_input_errors_when_thread_missing() {
vec![UserInput::Text {
text: "hello".to_string(),
text_elements: Vec::new(),
}],
}]
.into(),
)
.await
.expect_err("send_input should fail for missing thread");
@@ -387,7 +390,8 @@ async fn send_input_submits_user_message() {
vec![UserInput::Text {
text: "hello from tests".to_string(),
text_elements: Vec::new(),
}],
}]
.into(),
)
.await
.expect("send_input should succeed");
-17
View File
@@ -3980,17 +3980,6 @@ impl Session {
}
}
pub(crate) async fn pending_input_snapshot(&self) -> Vec<ResponseInputItem> {
let active = self.active_turn.lock().await;
match active.as_ref() {
Some(at) => {
let ts = at.turn_state.lock().await;
ts.pending_input_snapshot()
}
None => Vec::with_capacity(0),
}
}
/// 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() {
@@ -4005,12 +3994,6 @@ impl Session {
std::mem::take(&mut *self.idle_pending_input.lock().await)
}
pub(crate) async fn queued_response_items_for_next_turn_snapshot(
&self,
) -> Vec<ResponseInputItem> {
self.idle_pending_input.lock().await.clone()
}
pub(crate) async fn has_queued_response_items_for_next_turn(&self) -> bool {
!self.idle_pending_input.lock().await.is_empty()
}
+1 -1
View File
@@ -132,7 +132,7 @@ pub(super) async fn run(session: &Arc<Session>, config: Arc<Config>) {
let thread_id = match session
.services
.agent_control
.spawn_agent(agent_config, prompt, Some(source))
.spawn_agent(agent_config, prompt.into(), Some(source))
.await
{
Ok(thread_id) => thread_id,
@@ -2,13 +2,19 @@ use codex_protocol::config_types::CollaborationModeMask;
use codex_protocol::config_types::ModeKind;
use codex_protocol::config_types::TUI_VISIBLE_COLLABORATION_MODES;
use codex_protocol::openai_models::ReasoningEffort;
use codex_utils_template::Template;
use std::sync::LazyLock;
const COLLABORATION_MODE_PLAN: &str = include_str!("../../templates/collaboration_mode/plan.md");
const COLLABORATION_MODE_DEFAULT: &str =
include_str!("../../templates/collaboration_mode/default.md");
const KNOWN_MODE_NAMES_PLACEHOLDER: &str = "{{KNOWN_MODE_NAMES}}";
const REQUEST_USER_INPUT_AVAILABILITY_PLACEHOLDER: &str = "{{REQUEST_USER_INPUT_AVAILABILITY}}";
const ASKING_QUESTIONS_GUIDANCE_PLACEHOLDER: &str = "{{ASKING_QUESTIONS_GUIDANCE}}";
const KNOWN_MODE_NAMES_TEMPLATE_KEY: &str = "KNOWN_MODE_NAMES";
const REQUEST_USER_INPUT_AVAILABILITY_TEMPLATE_KEY: &str = "REQUEST_USER_INPUT_AVAILABILITY";
const ASKING_QUESTIONS_GUIDANCE_TEMPLATE_KEY: &str = "ASKING_QUESTIONS_GUIDANCE";
static COLLABORATION_MODE_DEFAULT_TEMPLATE: LazyLock<Template> = LazyLock::new(|| {
Template::parse(COLLABORATION_MODE_DEFAULT)
.unwrap_or_else(|err| panic!("collaboration mode default template must parse: {err}"))
});
/// Stores feature flags that control collaboration-mode behavior.
///
@@ -21,7 +27,7 @@ pub struct CollaborationModesConfig {
pub default_mode_request_user_input: bool,
}
pub(crate) fn builtin_collaboration_mode_presets(
pub fn builtin_collaboration_mode_presets(
collaboration_modes_config: CollaborationModesConfig,
) -> Vec<CollaborationModeMask> {
vec![plan_preset(), default_preset(collaboration_modes_config)]
@@ -56,16 +62,19 @@ fn default_mode_instructions(collaboration_modes_config: CollaborationModesConfi
let asking_questions_guidance = asking_questions_guidance_message(
collaboration_modes_config.default_mode_request_user_input,
);
COLLABORATION_MODE_DEFAULT
.replace(KNOWN_MODE_NAMES_PLACEHOLDER, &known_mode_names)
.replace(
REQUEST_USER_INPUT_AVAILABILITY_PLACEHOLDER,
&request_user_input_availability,
)
.replace(
ASKING_QUESTIONS_GUIDANCE_PLACEHOLDER,
&asking_questions_guidance,
)
COLLABORATION_MODE_DEFAULT_TEMPLATE
.render([
(KNOWN_MODE_NAMES_TEMPLATE_KEY, known_mode_names.as_str()),
(
REQUEST_USER_INPUT_AVAILABILITY_TEMPLATE_KEY,
request_user_input_availability.as_str(),
),
(
ASKING_QUESTIONS_GUIDANCE_TEMPLATE_KEY,
asking_questions_guidance.as_str(),
),
])
.unwrap_or_else(|err| panic!("collaboration mode default template must render: {err}"))
}
fn format_mode_names(modes: &[ModeKind]) -> String {
@@ -23,9 +23,9 @@ fn default_mode_instructions_replace_mode_names_placeholder() {
.expect("default preset should include instructions")
.expect("default instructions should be set");
assert!(!default_instructions.contains(KNOWN_MODE_NAMES_PLACEHOLDER));
assert!(!default_instructions.contains(REQUEST_USER_INPUT_AVAILABILITY_PLACEHOLDER));
assert!(!default_instructions.contains(ASKING_QUESTIONS_GUIDANCE_PLACEHOLDER));
assert!(!default_instructions.contains("{{KNOWN_MODE_NAMES}}"));
assert!(!default_instructions.contains("{{REQUEST_USER_INPUT_AVAILABILITY}}"));
assert!(!default_instructions.contains("{{ASKING_QUESTIONS_GUIDANCE}}"));
let known_mode_names = format_mode_names(&TUI_VISIBLE_COLLABORATION_MODES);
let expected_snippet = format!("Known mode names are {known_mode_names}.");
+105 -13
View File
@@ -1,7 +1,9 @@
use codex_git_utils::merge_base_with_head;
use codex_protocol::protocol::ReviewRequest;
use codex_protocol::protocol::ReviewTarget;
use codex_utils_template::Template;
use std::path::Path;
use std::sync::LazyLock;
#[derive(Clone, Debug, PartialEq)]
pub struct ResolvedReviewRequest {
@@ -12,12 +14,27 @@ pub struct ResolvedReviewRequest {
const UNCOMMITTED_PROMPT: &str = "Review the current code changes (staged, unstaged, and untracked files) and provide prioritized findings.";
const BASE_BRANCH_PROMPT_BACKUP: &str = "Review the code changes against the base branch '{branch}'. Start by finding the merge diff between the current branch and {branch}'s upstream e.g. (`git merge-base HEAD \"$(git rev-parse --abbrev-ref \"{branch}@{upstream}\")\"`), then run `git diff` against that SHA to see what changes we would merge into the {branch} branch. Provide prioritized, actionable findings.";
const BASE_BRANCH_PROMPT: &str = "Review the code changes against the base branch '{baseBranch}'. The merge base commit for this comparison is {mergeBaseSha}. Run `git diff {mergeBaseSha}` to inspect the changes relative to {baseBranch}. Provide prioritized, actionable findings.";
const BASE_BRANCH_PROMPT_BACKUP: &str = "Review the code changes against the base branch '{{branch}}'. Start by finding the merge diff between the current branch and {{branch}}'s upstream e.g. (`git merge-base HEAD \"$(git rev-parse --abbrev-ref \"{{branch}}@{upstream}\")\"`), then run `git diff` against that SHA to see what changes we would merge into the {{branch}} branch. Provide prioritized, actionable findings.";
const BASE_BRANCH_PROMPT: &str = "Review the code changes against the base branch '{{base_branch}}'. The merge base commit for this comparison is {{merge_base_sha}}. Run `git diff {{merge_base_sha}}` to inspect the changes relative to {{base_branch}}. Provide prioritized, actionable findings.";
static BASE_BRANCH_PROMPT_BACKUP_TEMPLATE: LazyLock<Template> = LazyLock::new(|| {
Template::parse(BASE_BRANCH_PROMPT_BACKUP)
.unwrap_or_else(|err| panic!("base branch backup review prompt must parse: {err}"))
});
static BASE_BRANCH_PROMPT_TEMPLATE: LazyLock<Template> = LazyLock::new(|| {
Template::parse(BASE_BRANCH_PROMPT)
.unwrap_or_else(|err| panic!("base branch review prompt must parse: {err}"))
});
const COMMIT_PROMPT_WITH_TITLE: &str = "Review the code changes introduced by commit {sha} (\"{title}\"). Provide prioritized, actionable findings.";
const COMMIT_PROMPT: &str =
"Review the code changes introduced by commit {sha}. Provide prioritized, actionable findings.";
const COMMIT_PROMPT_WITH_TITLE: &str = "Review the code changes introduced by commit {{sha}} (\"{{title}}\"). Provide prioritized, actionable findings.";
const COMMIT_PROMPT: &str = "Review the code changes introduced by commit {{sha}}. Provide prioritized, actionable findings.";
static COMMIT_PROMPT_WITH_TITLE_TEMPLATE: LazyLock<Template> = LazyLock::new(|| {
Template::parse(COMMIT_PROMPT_WITH_TITLE)
.unwrap_or_else(|err| panic!("commit review prompt with title must parse: {err}"))
});
static COMMIT_PROMPT_TEMPLATE: LazyLock<Template> = LazyLock::new(|| {
Template::parse(COMMIT_PROMPT)
.unwrap_or_else(|err| panic!("commit review prompt must parse: {err}"))
});
pub fn resolve_review_request(
request: ReviewRequest,
@@ -41,20 +58,31 @@ pub fn review_prompt(target: &ReviewTarget, cwd: &Path) -> anyhow::Result<String
ReviewTarget::UncommittedChanges => Ok(UNCOMMITTED_PROMPT.to_string()),
ReviewTarget::BaseBranch { branch } => {
if let Some(commit) = merge_base_with_head(cwd, branch)? {
Ok(BASE_BRANCH_PROMPT
.replace("{baseBranch}", branch)
.replace("{mergeBaseSha}", &commit))
Ok(render_review_prompt(
&BASE_BRANCH_PROMPT_TEMPLATE,
[
("base_branch", branch.as_str()),
("merge_base_sha", commit.as_str()),
],
))
} else {
Ok(BASE_BRANCH_PROMPT_BACKUP.replace("{branch}", branch))
Ok(render_review_prompt(
&BASE_BRANCH_PROMPT_BACKUP_TEMPLATE,
[("branch", branch.as_str())],
))
}
}
ReviewTarget::Commit { sha, title } => {
if let Some(title) = title {
Ok(COMMIT_PROMPT_WITH_TITLE
.replace("{sha}", sha)
.replace("{title}", title))
Ok(render_review_prompt(
&COMMIT_PROMPT_WITH_TITLE_TEMPLATE,
[("sha", sha.as_str()), ("title", title.as_str())],
))
} else {
Ok(COMMIT_PROMPT.replace("{sha}", sha))
Ok(render_review_prompt(
&COMMIT_PROMPT_TEMPLATE,
[("sha", sha.as_str())],
))
}
}
ReviewTarget::Custom { instructions } => {
@@ -67,6 +95,15 @@ pub fn review_prompt(target: &ReviewTarget, cwd: &Path) -> anyhow::Result<String
}
}
fn render_review_prompt<'a, const N: usize>(
template: &Template,
variables: [(&'a str, &'a str); N],
) -> String {
template
.render(variables)
.unwrap_or_else(|err| panic!("review prompt template must render: {err}"))
}
pub fn user_facing_hint(target: &ReviewTarget) -> String {
match target {
ReviewTarget::UncommittedChanges => "current changes".to_string(),
@@ -91,3 +128,58 @@ impl From<ResolvedReviewRequest> for ReviewRequest {
}
}
}
#[cfg(test)]
mod tests {
use super::*;
use pretty_assertions::assert_eq;
#[test]
fn review_prompt_template_renders_base_branch_backup_variant() {
assert_eq!(
render_review_prompt(&BASE_BRANCH_PROMPT_BACKUP_TEMPLATE, [("branch", "main")]),
"Review the code changes against the base branch 'main'. Start by finding the merge diff between the current branch and main's upstream e.g. (`git merge-base HEAD \"$(git rev-parse --abbrev-ref \"main@{upstream}\")\"`), then run `git diff` against that SHA to see what changes we would merge into the main branch. Provide prioritized, actionable findings."
);
}
#[test]
fn review_prompt_template_renders_base_branch_variant() {
assert_eq!(
render_review_prompt(
&BASE_BRANCH_PROMPT_TEMPLATE,
[("base_branch", "main"), ("merge_base_sha", "abc123")]
),
"Review the code changes against the base branch 'main'. The merge base commit for this comparison is abc123. Run `git diff abc123` to inspect the changes relative to main. Provide prioritized, actionable findings."
);
}
#[test]
fn review_prompt_template_renders_commit_variant() {
assert_eq!(
review_prompt(
&ReviewTarget::Commit {
sha: "deadbeef".to_string(),
title: None,
},
Path::new("."),
)
.expect("commit prompt should render"),
"Review the code changes introduced by commit deadbeef. Provide prioritized, actionable findings."
);
}
#[test]
fn review_prompt_template_renders_commit_variant_with_title() {
assert_eq!(
review_prompt(
&ReviewTarget::Commit {
sha: "deadbeef".to_string(),
title: Some("Fix bug".to_string()),
},
Path::new("."),
)
.expect("commit prompt should render"),
"Review the code changes introduced by commit deadbeef (\"Fix bug\"). Provide prioritized, actionable findings."
);
}
}
-4
View File
@@ -198,10 +198,6 @@ impl TurnState {
}
}
pub(crate) fn pending_input_snapshot(&self) -> Vec<ResponseInputItem> {
self.pending_input.clone()
}
pub(crate) fn has_pending_input(&self) -> bool {
!self.pending_input.is_empty()
}
+28 -2
View File
@@ -14,6 +14,7 @@ use codex_protocol::protocol::ExitedReviewModeEvent;
use codex_protocol::protocol::ItemCompletedEvent;
use codex_protocol::protocol::ReviewOutputEvent;
use codex_protocol::protocol::SubAgentSource;
use codex_utils_template::Template;
use tokio_util::sync::CancellationToken;
use crate::codex::Session;
@@ -25,10 +26,16 @@ use crate::review_format::render_review_output_text;
use crate::state::TaskKind;
use codex_features::Feature;
use codex_protocol::user_input::UserInput;
use std::sync::LazyLock;
use super::SessionTask;
use super::SessionTaskContext;
static REVIEW_EXIT_SUCCESS_TEMPLATE: LazyLock<Template> = LazyLock::new(|| {
Template::parse(crate::client_common::REVIEW_EXIT_SUCCESS_TMPL)
.unwrap_or_else(|err| panic!("review exit success template must parse: {err}"))
});
#[derive(Clone, Copy)]
pub(crate) struct ReviewTask;
@@ -220,8 +227,7 @@ pub(crate) async fn exit_review_mode(
let block = format_review_findings_block(&out.findings, /*selection*/ None);
findings_str.push_str(&format!("\n{block}"));
}
let rendered =
crate::client_common::REVIEW_EXIT_SUCCESS_TMPL.replace("{results}", &findings_str);
let rendered = render_review_exit_success(&findings_str);
let assistant_message = render_review_output_text(&out);
(rendered, assistant_message)
} else {
@@ -271,3 +277,23 @@ pub(crate) async fn exit_review_mode(
// file creation + git metadata collection cannot delay client-facing items.
session.ensure_rollout_materialized().await;
}
fn render_review_exit_success(results: &str) -> String {
REVIEW_EXIT_SUCCESS_TEMPLATE
.render([("results", results)])
.unwrap_or_else(|err| panic!("review exit success template must render: {err}"))
}
#[cfg(test)]
mod tests {
use super::render_review_exit_success;
use pretty_assertions::assert_eq;
#[test]
fn render_review_exit_success_replaces_results_placeholder() {
assert_eq!(
render_review_exit_success("Finding A\nFinding B"),
"<user_action>\n <context>User initiated a review task. Here's the full review output from reviewer model. User may select one or more comments to resolve.</context>\n <action>review</action>\n <results>\n Finding A\nFinding B\n </results>\n </user_action>\n"
);
}
}
@@ -633,7 +633,7 @@ async fn run_agent_job_loop(
.agent_control
.spawn_agent(
options.spawn_config.clone(),
items,
items.into(),
Some(SessionSource::SubAgent(SubAgentSource::Other(format!(
"agent_job:{job_id}"
)))),
@@ -1,4 +1,5 @@
use super::*;
use crate::agent::control::render_input_preview;
pub(crate) struct Handler;
@@ -26,7 +27,7 @@ impl ToolHandler for Handler {
let args: SendInputArgs = parse_arguments(&arguments)?;
let receiver_thread_id = parse_agent_id_target(&args.target)?;
let input_items = parse_collab_input(args.message, args.items)?;
let prompt = input_preview(&input_items);
let prompt = render_input_preview(&input_items);
let receiver_agent = session
.services
.agent_control
@@ -1,5 +1,6 @@
use super::*;
use crate::agent::control::SpawnAgentOptions;
use crate::agent::control::render_input_preview;
use crate::agent::role::DEFAULT_ROLE_NAME;
use crate::agent::role::apply_role_to_config;
@@ -36,7 +37,7 @@ impl ToolHandler for Handler {
.map(str::trim)
.filter(|role| !role.is_empty());
let input_items = parse_collab_input(args.message, args.items)?;
let prompt = input_preview(&input_items);
let prompt = render_input_preview(&input_items);
let session_source = turn.session_source.clone();
let child_depth = next_thread_spawn_depth(&session_source);
let max_depth = turn.config.agent_max_depth;
@@ -17,6 +17,7 @@ use codex_protocol::openai_models::ReasoningEffort;
use codex_protocol::openai_models::ReasoningEffortPreset;
use codex_protocol::protocol::CollabAgentRef;
use codex_protocol::protocol::CollabAgentStatusEntry;
use codex_protocol::protocol::Op;
use codex_protocol::protocol::SessionSource;
use codex_protocol::protocol::SubAgentSource;
use codex_protocol::user_input::UserInput;
@@ -161,7 +162,7 @@ pub(crate) fn thread_spawn_source(
pub(crate) fn parse_collab_input(
message: Option<String>,
items: Option<Vec<UserInput>>,
) -> Result<Vec<UserInput>, FunctionCallError> {
) -> Result<Op, FunctionCallError> {
match (message, items) {
(Some(_), Some(_)) => Err(FunctionCallError::RespondToModel(
"Provide either message or items, but not both".to_string(),
@@ -178,7 +179,8 @@ pub(crate) fn parse_collab_input(
Ok(vec![UserInput::Text {
text: message,
text_elements: Vec::new(),
}])
}]
.into())
}
(None, Some(items)) => {
if items.is_empty() {
@@ -186,29 +188,11 @@ pub(crate) fn parse_collab_input(
"Items can't be empty".to_string(),
));
}
Ok(items)
Ok(items.into())
}
}
}
pub(crate) fn input_preview(items: &[UserInput]) -> String {
let parts: Vec<String> = items
.iter()
.map(|item| match item {
UserInput::Text { text, .. } => text.clone(),
UserInput::Image { .. } => "[image]".to_string(),
UserInput::LocalImage { path } => format!("[local_image:{}]", path.display()),
UserInput::Skill { name, path } => {
format!("[skill:${name}]({})", path.display())
}
UserInput::Mention { name, path } => format!("[mention:${name}]({path})"),
_ => "[input]".to_string(),
})
.collect();
parts.join("\n")
}
/// Builds the base config snapshot for a newly spawned sub-agent.
///
/// The returned config starts from the parent's effective config and then refreshes the
@@ -321,6 +321,40 @@ async fn spawn_agent_returns_agent_id_without_task_name() {
assert_eq!(success, Some(true));
}
#[tokio::test]
async fn multi_agent_v2_spawn_requires_task_name() {
let (mut session, mut turn) = make_session_and_context().await;
let manager = thread_manager();
let root = manager
.start_thread((*turn.config).clone())
.await
.expect("root thread should start");
session.services.agent_control = manager.agent_control();
session.conversation_id = root.thread_id;
let mut config = (*turn.config).clone();
config
.features
.enable(Feature::MultiAgentV2)
.expect("test config should allow feature update");
turn.config = Arc::new(config);
let invocation = invocation(
Arc::new(session),
Arc::new(turn),
"spawn_agent",
function_payload(json!({
"message": "inspect this repo"
})),
);
let Err(err) = SpawnAgentHandlerV2.handle(invocation).await else {
panic!("missing task_name should be rejected");
};
let FunctionCallError::RespondToModel(message) = err else {
panic!("missing task_name should surface as a model-facing error");
};
assert!(message.contains("missing field `task_name`"));
}
#[tokio::test]
async fn spawn_agent_errors_when_manager_dropped() {
let (session, turn) = make_session_and_context().await;
@@ -402,6 +436,18 @@ async fn multi_agent_v2_spawn_returns_path_and_send_message_accepts_relative_pat
child_snapshot.session_source.get_agent_path().as_deref(),
Some("/root/test_process")
);
assert!(manager.captured_ops().iter().any(|(id, op)| {
*id == child_thread_id
&& matches!(
op,
Op::InterAgentCommunication { communication }
if communication.author == AgentPath::root()
&& communication.recipient.as_str() == "/root/test_process"
&& communication.other_recipients.is_empty()
&& communication.content == "inspect this repo"
&& communication.trigger_turn
)
}));
SendMessageHandlerV2
.handle(invocation(
@@ -456,7 +502,8 @@ async fn multi_agent_v2_send_message_accepts_root_target_from_child() {
vec![UserInput::Text {
text: "inspect this repo".to_string(),
text_elements: Vec::new(),
}],
}]
.into(),
Some(SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
parent_thread_id: root.thread_id,
depth: 1,
@@ -620,7 +667,8 @@ async fn multi_agent_v2_list_agents_filters_by_relative_path_prefix() {
vec![UserInput::Text {
text: "research".to_string(),
text_elements: Vec::new(),
}],
}]
.into(),
Some(SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
parent_thread_id: root.thread_id,
depth: 1,
@@ -640,7 +688,8 @@ async fn multi_agent_v2_list_agents_filters_by_relative_path_prefix() {
vec![UserInput::Text {
text: "build".to_string(),
text_elements: Vec::new(),
}],
}]
.into(),
Some(SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
parent_thread_id: root.thread_id,
depth: 2,
@@ -4,6 +4,7 @@
//! whether the resulting `InterAgentCommunication` should wake the target immediately.
use super::*;
use crate::agent::control::render_input_preview;
use codex_protocol::protocol::InterAgentCommunication;
#[derive(Clone, Copy)]
@@ -83,7 +84,7 @@ fn text_content(
.iter()
.all(|item| matches!(item, UserInput::Text { .. }))
{
return Ok(input_preview(items));
return Ok(render_input_preview(&(items.to_vec().into())));
}
Err(FunctionCallError::RespondToModel(
mode.unsupported_items_error().to_string(),
@@ -1,8 +1,12 @@
use super::*;
use crate::agent::control::SpawnAgentOptions;
use crate::agent::control::render_input_preview;
use crate::agent::next_thread_spawn_depth;
use crate::agent::role::DEFAULT_ROLE_NAME;
use crate::agent::role::apply_role_to_config;
use codex_protocol::AgentPath;
use codex_protocol::protocol::InterAgentCommunication;
use codex_protocol::protocol::Op;
pub(crate) struct Handler;
@@ -33,8 +37,10 @@ impl ToolHandler for Handler {
.as_deref()
.map(str::trim)
.filter(|role| !role.is_empty());
let input_items = parse_collab_input(args.message, args.items)?;
let prompt = input_preview(&input_items);
let initial_operation = parse_collab_input(args.message, args.items)?;
let prompt = render_input_preview(&initial_operation);
let session_source = turn.session_source.clone();
let child_depth = next_thread_spawn_depth(&session_source);
let max_depth = turn.config.agent_max_depth;
@@ -72,19 +78,39 @@ impl ToolHandler for Handler {
apply_spawn_agent_runtime_overrides(&mut config, turn.as_ref())?;
apply_spawn_agent_overrides(&mut config, child_depth);
let spawn_source = thread_spawn_source(
session.conversation_id,
&turn.session_source,
child_depth,
role_name,
Some(args.task_name.clone()),
)?;
let result = session
.services
.agent_control
.spawn_agent_with_metadata(
config,
input_items,
Some(thread_spawn_source(
session.conversation_id,
&turn.session_source,
child_depth,
role_name,
args.task_name.clone(),
)?),
match (spawn_source.get_agent_path(), initial_operation) {
(Some(recipient), Op::UserInput { items, .. })
if items
.iter()
.all(|item| matches!(item, UserInput::Text { .. })) =>
{
Op::InterAgentCommunication {
communication: InterAgentCommunication::new(
turn.session_source
.get_agent_path()
.unwrap_or_else(AgentPath::root),
recipient,
Vec::new(),
prompt.clone(),
/*trigger_turn*/ true,
),
}
}
(_, initial_operation) => initial_operation,
},
Some(spawn_source),
SpawnAgentOptions {
fork_parent_spawn_call_id: args.fork_context.then(|| call_id.clone()),
},
@@ -132,7 +158,6 @@ impl ToolHandler for Handler {
.and_then(|snapshot| snapshot.reasoning_effort)
.unwrap_or(args.reasoning_effort.unwrap_or_default());
let nickname = new_agent_nickname.clone();
let task_name = new_agent_path.clone();
session
.send_event(
&turn,
@@ -150,16 +175,21 @@ impl ToolHandler for Handler {
.into(),
)
.await;
let new_thread_id = result?.thread_id;
let _ = result?;
let role_tag = role_name.unwrap_or(DEFAULT_ROLE_NAME);
turn.session_telemetry.counter(
"codex.multi_agent.spawn",
/*inc*/ 1,
&[("role", role_tag)],
);
let task_name = new_agent_path.ok_or_else(|| {
FunctionCallError::RespondToModel(
"spawned agent is missing a canonical task name".to_string(),
)
})?;
Ok(SpawnAgentResult {
agent_id: task_name.is_none().then(|| new_thread_id.to_string()),
agent_id: None,
task_name,
nickname,
})
@@ -170,7 +200,7 @@ impl ToolHandler for Handler {
struct SpawnAgentArgs {
message: Option<String>,
items: Option<Vec<UserInput>>,
task_name: Option<String>,
task_name: String,
agent_type: Option<String>,
model: Option<String>,
reasoning_effort: Option<ReasoningEffort>,
@@ -181,7 +211,7 @@ struct SpawnAgentArgs {
#[derive(Debug, Serialize)]
pub(crate) struct SpawnAgentResult {
agent_id: Option<String>,
task_name: Option<String>,
task_name: String,
nickname: Option<String>,
}
+50 -42
View File
@@ -46,9 +46,11 @@ use codex_protocol::openai_models::WebSearchToolType;
use codex_protocol::protocol::SandboxPolicy;
use codex_protocol::protocol::SessionSource;
use codex_protocol::protocol::SubAgentSource;
use codex_tools::ToolDefinition;
use codex_tools::parse_dynamic_tool;
use codex_tools::parse_mcp_tool;
pub use codex_tools::parse_tool_input_schema;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_template::Template;
use serde::Deserialize;
use serde::Serialize;
use serde_json::Value as JsonValue;
@@ -56,16 +58,27 @@ use serde_json::json;
use std::collections::BTreeMap;
use std::collections::HashMap;
use std::path::PathBuf;
use std::sync::LazyLock;
pub type JsonSchema = codex_tools::JsonSchema;
#[cfg(test)]
pub(crate) use codex_tools::mcp_call_tool_result_output_schema;
const TOOL_SEARCH_DESCRIPTION_TEMPLATE: &str =
const TOOL_SEARCH_DESCRIPTION_TEMPLATE_SOURCE: &str =
include_str!("../../templates/search_tool/tool_description.md");
const TOOL_SUGGEST_DESCRIPTION_TEMPLATE: &str =
const TOOL_SEARCH_DESCRIPTION_TEMPLATE_KEY: &str = "app_descriptions";
static TOOL_SEARCH_DESCRIPTION_TEMPLATE: LazyLock<Template> = LazyLock::new(|| {
Template::parse(TOOL_SEARCH_DESCRIPTION_TEMPLATE_SOURCE)
.unwrap_or_else(|err| panic!("tool_search description template must parse: {err}"))
});
const TOOL_SUGGEST_DESCRIPTION_TEMPLATE_SOURCE: &str =
include_str!("../../templates/search_tool/tool_suggest_description.md");
const TOOL_SUGGEST_DESCRIPTION_TEMPLATE_KEY: &str = "discoverable_tools";
static TOOL_SUGGEST_DESCRIPTION_TEMPLATE: LazyLock<Template> = LazyLock::new(|| {
Template::parse(TOOL_SUGGEST_DESCRIPTION_TEMPLATE_SOURCE)
.unwrap_or_else(|err| panic!("tool_suggest description template must parse: {err}"))
});
const WEB_SEARCH_CONTENT_TYPES: [&str; 2] = ["text", "image"];
fn unified_exec_output_schema() -> JsonValue {
@@ -157,10 +170,10 @@ fn spawn_agent_output_schema_v2() -> JsonValue {
"properties": {
"agent_id": {
"type": ["string", "null"],
"description": "Thread identifier for the spawned agent when no task name was assigned."
"description": "Legacy thread identifier for the spawned agent."
},
"task_name": {
"type": ["string", "null"],
"type": "string",
"description": "Canonical task name for the spawned agent."
},
"nickname": {
@@ -1215,13 +1228,13 @@ fn create_spawn_agent_tool_v1(config: &ToolsConfig) -> ToolSpec {
fn create_spawn_agent_tool_v2(config: &ToolsConfig) -> ToolSpec {
let available_models_description = spawn_agent_models_description(&config.available_models);
let return_value_description = "Returns the canonical task name when the spawned agent was named, otherwise the agent id, plus the user-facing nickname when available.";
let return_value_description = "Returns the canonical task name for the spawned agent, plus the user-facing nickname when available.";
let mut properties = spawn_agent_common_properties(config);
properties.insert(
"task_name".to_string(),
JsonSchema::String {
description: Some(
"Optional task name for the new agent. Use lowercase letters, digits, and underscores."
"Task name for the new agent. Use lowercase letters, digits, and underscores."
.to_string(),
),
},
@@ -1237,7 +1250,7 @@ fn create_spawn_agent_tool_v2(config: &ToolsConfig) -> ToolSpec {
defer_loading: None,
parameters: JsonSchema::Object {
properties,
required: None,
required: Some(vec!["task_name".to_string()]),
additional_properties: Some(false.into()),
},
output_schema: Some(spawn_agent_output_schema_v2()),
@@ -1950,8 +1963,12 @@ fn create_tool_search_tool(app_tools: &HashMap<String, ToolInfo>) -> ToolSpec {
.join("\n")
};
let description =
TOOL_SEARCH_DESCRIPTION_TEMPLATE.replace("{{app_descriptions}}", app_descriptions.as_str());
let description = TOOL_SEARCH_DESCRIPTION_TEMPLATE
.render([(
TOOL_SEARCH_DESCRIPTION_TEMPLATE_KEY,
app_descriptions.as_str(),
)])
.unwrap_or_else(|err| panic!("tool_search description template must render: {err}"));
ToolSpec::ToolSearch {
execution: "client".to_string(),
@@ -2006,10 +2023,13 @@ fn create_tool_suggest_tool(discoverable_tools: &[DiscoverableTool]) -> ToolSpec
},
),
]);
let description = TOOL_SUGGEST_DESCRIPTION_TEMPLATE.replace(
"{{discoverable_tools}}",
format_discoverable_tools(discoverable_tools).as_str(),
);
let discoverable_tools = format_discoverable_tools(discoverable_tools);
let description = TOOL_SUGGEST_DESCRIPTION_TEMPLATE
.render([(
TOOL_SUGGEST_DESCRIPTION_TEMPLATE_KEY,
discoverable_tools.as_str(),
)])
.unwrap_or_else(|err| panic!("tool_suggest description template must render: {err}"));
ToolSpec::Function(ResponsesApiTool {
name: TOOL_SUGGEST_TOOL_NAME.to_string(),
@@ -2366,47 +2386,35 @@ pub(crate) fn mcp_tool_to_openai_tool(
fully_qualified_name: String,
tool: rmcp::model::Tool,
) -> Result<ResponsesApiTool, serde_json::Error> {
let parsed_tool = parse_mcp_tool(&tool)?;
Ok(ResponsesApiTool {
name: fully_qualified_name,
description: parsed_tool.description,
strict: false,
defer_loading: None,
parameters: parsed_tool.input_schema,
output_schema: Some(parsed_tool.output_schema),
})
Ok(tool_definition_to_openai_tool(
parse_mcp_tool(&tool)?.renamed(fully_qualified_name),
))
}
pub(crate) fn mcp_tool_to_deferred_openai_tool(
name: String,
tool: rmcp::model::Tool,
) -> Result<ResponsesApiTool, serde_json::Error> {
let parsed_tool = parse_mcp_tool(&tool)?;
Ok(ResponsesApiTool {
name,
description: parsed_tool.description,
strict: false,
defer_loading: Some(true),
parameters: parsed_tool.input_schema,
output_schema: None,
})
Ok(tool_definition_to_openai_tool(
parse_mcp_tool(&tool)?.renamed(name).into_deferred(),
))
}
fn dynamic_tool_to_openai_tool(
tool: &DynamicToolSpec,
) -> Result<ResponsesApiTool, serde_json::Error> {
let input_schema = parse_tool_input_schema(&tool.input_schema)?;
Ok(tool_definition_to_openai_tool(parse_dynamic_tool(tool)?))
}
Ok(ResponsesApiTool {
name: tool.name.clone(),
description: tool.description.clone(),
fn tool_definition_to_openai_tool(tool_definition: ToolDefinition) -> ResponsesApiTool {
ResponsesApiTool {
name: tool_definition.name,
description: tool_definition.description,
strict: false,
defer_loading: None,
parameters: input_schema,
output_schema: None,
})
defer_loading: tool_definition.defer_loading.then_some(true),
parameters: tool_definition.input_schema,
output_schema: tool_definition.output_schema,
}
}
/// Builds the tool registry builder while collecting tool specs for later serialization.
+41 -1
View File
@@ -8,6 +8,7 @@ use crate::tools::ToolRouter;
use crate::tools::registry::ConfiguredToolSpec;
use crate::tools::router::ToolRouterParams;
use codex_app_server_protocol::AppInfo;
use codex_protocol::dynamic_tools::DynamicToolSpec;
use codex_protocol::openai_models::InputModality;
use codex_protocol::openai_models::ModelInfo;
use codex_protocol::openai_models::ModelsResponse;
@@ -126,6 +127,44 @@ fn deferred_responses_api_tool_serializes_with_defer_loading() {
);
}
#[test]
fn dynamic_tool_preserves_defer_loading() {
let tool = DynamicToolSpec {
name: "lookup_order".to_string(),
description: "Look up an order".to_string(),
input_schema: serde_json::json!({
"type": "object",
"properties": {
"order_id": {"type": "string"}
},
"required": ["order_id"],
"additionalProperties": false,
}),
defer_loading: true,
};
let openai_tool = dynamic_tool_to_openai_tool(&tool).expect("convert dynamic tool");
assert_eq!(
openai_tool,
ResponsesApiTool {
name: "lookup_order".to_string(),
description: "Look up an order".to_string(),
strict: false,
defer_loading: Some(true),
parameters: JsonSchema::Object {
properties: BTreeMap::from([(
"order_id".to_string(),
JsonSchema::String { description: None },
)]),
required: Some(vec!["order_id".to_string()]),
additional_properties: Some(false.into()),
},
output_schema: None,
}
);
}
fn tool_name(tool: &ToolSpec) -> &str {
match tool {
ToolSpec::Function(ResponsesApiTool { name, .. }) => name,
@@ -454,7 +493,7 @@ fn test_build_specs_multi_agent_v2_uses_task_names_and_hides_resume() {
panic!("spawn_agent should use object params");
};
assert!(properties.contains_key("task_name"));
assert_eq!(required.as_ref(), None);
assert_eq!(required.as_ref(), Some(&vec!["task_name".to_string()]));
let output_schema = output_schema
.as_ref()
.expect("spawn_agent should define output schema");
@@ -2242,6 +2281,7 @@ fn tool_suggest_description_lists_discoverable_tools() {
"Apply the stricter explicit-and-unambiguous rule for *discoverable tools* like plugin install suggestions; *missing tools* like connector install suggestions continue to use the normal clear-fit standard."
));
assert!(description.contains("DO NOT explore or recommend tools that are not on this list."));
assert!(!description.contains("{{discoverable_tools}}"));
assert!(!description.contains("tool_search fails to find a good match"));
let JsonSchema::Object { required, .. } = parameters else {
panic!("expected object parameters");
@@ -2,7 +2,6 @@
<context>User initiated a review task. Here's the full review output from reviewer model. User may select one or more comments to resolve.</context>
<action>review</action>
<results>
{results}
{{results}}
</results>
</user_action>
+1 -1
View File
@@ -179,7 +179,7 @@ impl ActionKind {
Ok((event, Some(command)))
}
ActionKind::RunCommand { command } => {
let event = shell_event(call_id, command, 1_000, sandbox_permissions)?;
let event = shell_event(call_id, command, 2_000, sandbox_permissions)?;
Ok((event, Some(command.to_string())))
}
ActionKind::RunUnifiedExecCommand {
+1
View File
@@ -17,6 +17,7 @@ codex-config = { workspace = true }
codex-keyring-store = { workspace = true }
codex-protocol = { workspace = true }
codex-terminal-detection = { workspace = true }
codex-utils-template = { workspace = true }
once_cell = { workspace = true }
os_info = { workspace = true }
rand = { workspace = true }
+5 -5
View File
@@ -101,21 +101,21 @@
<div class="brand-title">Codex login</div>
</div>
<h1>__ERROR_TITLE__</h1>
<p class="message">__ERROR_MESSAGE__</p>
<h1>{{error_title}}</h1>
<p class="message">{{error_message}}</p>
<div class="details">
<div class="details-row">
<strong>Error code</strong>
<code>__ERROR_CODE__</code>
<code>{{error_code}}</code>
</div>
<div class="details-row">
<strong>Details</strong>
<code>__ERROR_DESCRIPTION__</code>
<code>{{error_description}}</code>
</div>
</div>
<p class="help">__ERROR_HELP__</p>
<p class="help">{{error_help}}</p>
</div>
</div>
</body>
+53 -7
View File
@@ -20,6 +20,7 @@ use std::net::TcpStream;
use std::path::Path;
use std::path::PathBuf;
use std::sync::Arc;
use std::sync::LazyLock;
use std::thread;
use std::time::Duration;
@@ -35,6 +36,7 @@ use base64::Engine;
use chrono::Utc;
use codex_app_server_protocol::AuthMode;
use codex_client::build_reqwest_client_with_custom_ca;
use codex_utils_template::Template;
use rand::RngCore;
use serde_json::Value as JsonValue;
use tiny_http::Header;
@@ -48,6 +50,10 @@ use tracing::warn;
const DEFAULT_ISSUER: &str = "https://auth.openai.com";
const DEFAULT_PORT: u16 = 1455;
static LOGIN_ERROR_PAGE_TEMPLATE: LazyLock<Template> = LazyLock::new(|| {
Template::parse(include_str!("assets/error.html"))
.unwrap_or_else(|err| panic!("login error page template must parse: {err}"))
});
/// Options for launching the local login callback server.
#[derive(Debug, Clone)]
@@ -1001,7 +1007,6 @@ fn render_login_error_page(
error_code: Option<&str>,
error_description: Option<&str>,
) -> Vec<u8> {
let template = include_str!("assets/error.html");
let code = error_code.unwrap_or("unknown_error");
let (title, display_message, display_description, help_text) =
if is_missing_codex_entitlement_error(code, error_description) {
@@ -1022,12 +1027,15 @@ fn render_login_error_page(
.to_string(),
)
};
template
.replace("__ERROR_TITLE__", &html_escape(&title))
.replace("__ERROR_MESSAGE__", &html_escape(&display_message))
.replace("__ERROR_CODE__", &html_escape(code))
.replace("__ERROR_DESCRIPTION__", &html_escape(&display_description))
.replace("__ERROR_HELP__", &html_escape(&help_text))
LOGIN_ERROR_PAGE_TEMPLATE
.render([
("error_title", html_escape(&title)),
("error_message", html_escape(&display_message)),
("error_code", html_escape(code)),
("error_description", html_escape(&display_description)),
("error_help", html_escape(&help_text)),
])
.unwrap_or_else(|err| panic!("login error page template must render: {err}"))
.into_bytes()
}
@@ -1087,9 +1095,12 @@ mod tests {
use pretty_assertions::assert_eq;
use super::TokenEndpointErrorDetail;
use super::html_escape;
use super::is_missing_codex_entitlement_error;
use super::parse_token_endpoint_error;
use super::redact_sensitive_query_value;
use super::redact_sensitive_url_parts;
use super::render_login_error_page;
use super::sanitize_url_for_logging;
#[test]
@@ -1189,4 +1200,39 @@ mod tests {
"https://example.com/base?token=%3Credacted%3E&env=prod".to_string()
);
}
#[test]
fn render_login_error_page_escapes_dynamic_fields() {
let body = String::from_utf8(render_login_error_page(
"<bad>",
Some("code&value"),
Some("\"quoted\""),
))
.expect("login error page should be utf-8");
assert!(body.contains(&html_escape("Sign-in could not be completed")));
assert!(body.contains("&lt;bad&gt;"));
assert!(body.contains("code&amp;value"));
assert!(body.contains("&quot;quoted&quot;"));
}
#[test]
fn render_login_error_page_uses_entitlement_copy() {
let error_description = Some("missing_codex_entitlement");
assert!(is_missing_codex_entitlement_error(
"access_denied",
error_description
));
let body = String::from_utf8(render_login_error_page(
"access denied",
Some("access_denied"),
error_description,
))
.expect("login error page should be utf-8");
assert!(body.contains("You do not have access to Codex"));
assert!(body.contains("Contact your workspace administrator"));
assert!(!body.contains("missing_codex_entitlement"));
}
}
+1
View File
@@ -17,6 +17,7 @@ codex-git-utils = { workspace = true }
codex-utils-absolute-path = { workspace = true }
codex-utils-image = { workspace = true }
codex-utils-string = { workspace = true }
codex-utils-template = { workspace = true }
icu_decimal = { workspace = true }
icu_locale_core = { workspace = true }
icu_provider = { workspace = true, features = ["sync"] }
+30 -4
View File
@@ -1,8 +1,10 @@
use std::collections::HashMap;
use std::path::Path;
use std::sync::LazyLock;
use codex_utils_image::PromptImageMode;
use codex_utils_image::load_for_prompt_bytes;
use codex_utils_template::Template;
use serde::Deserialize;
use serde::Deserializer;
use serde::Serialize;
@@ -30,6 +32,19 @@ use schemars::JsonSchema;
use crate::mcp::CallToolResult;
static SANDBOX_MODE_DANGER_FULL_ACCESS_TEMPLATE: LazyLock<Template> = LazyLock::new(|| {
Template::parse(SANDBOX_MODE_DANGER_FULL_ACCESS.trim_end())
.unwrap_or_else(|err| panic!("danger-full-access sandbox template must parse: {err}"))
});
static SANDBOX_MODE_WORKSPACE_WRITE_TEMPLATE: LazyLock<Template> = LazyLock::new(|| {
Template::parse(SANDBOX_MODE_WORKSPACE_WRITE.trim_end())
.unwrap_or_else(|err| panic!("workspace-write sandbox template must parse: {err}"))
});
static SANDBOX_MODE_READ_ONLY_TEMPLATE: LazyLock<Template> = LazyLock::new(|| {
Template::parse(SANDBOX_MODE_READ_ONLY.trim_end())
.unwrap_or_else(|err| panic!("read-only sandbox template must parse: {err}"))
});
/// Controls the per-command sandbox override requested by a shell-like tool call.
#[derive(
Debug, Clone, Copy, Default, Eq, Hash, PartialEq, Serialize, Deserialize, JsonSchema, TS,
@@ -583,11 +598,14 @@ impl DeveloperInstructions {
fn sandbox_text(mode: SandboxMode, network_access: NetworkAccess) -> DeveloperInstructions {
let template = match mode {
SandboxMode::DangerFullAccess => SANDBOX_MODE_DANGER_FULL_ACCESS.trim_end(),
SandboxMode::WorkspaceWrite => SANDBOX_MODE_WORKSPACE_WRITE.trim_end(),
SandboxMode::ReadOnly => SANDBOX_MODE_READ_ONLY.trim_end(),
SandboxMode::DangerFullAccess => &*SANDBOX_MODE_DANGER_FULL_ACCESS_TEMPLATE,
SandboxMode::WorkspaceWrite => &*SANDBOX_MODE_WORKSPACE_WRITE_TEMPLATE,
SandboxMode::ReadOnly => &*SANDBOX_MODE_READ_ONLY_TEMPLATE,
};
let text = template.replace("{network_access}", &network_access.to_string());
let network_access = network_access.to_string();
let text = template
.render([("network_access", network_access.as_str())])
.unwrap_or_else(|err| panic!("sandbox template must render: {err}"));
DeveloperInstructions::new(text)
}
@@ -1654,6 +1672,14 @@ mod tests {
"Filesystem sandboxing defines which files can be read or written. `sandbox_mode` is `read-only`: The sandbox only permits reading files. Network access is restricted."
)
);
let danger_full_access: DeveloperInstructions = SandboxMode::DangerFullAccess.into();
assert_eq!(
danger_full_access,
DeveloperInstructions::new(
"Filesystem sandboxing defines which files can be read or written. `sandbox_mode` is `danger-full-access`: No filesystem sandboxing - all commands are permitted. Network access is enabled."
)
);
}
#[test]
@@ -1 +1 @@
Filesystem sandboxing defines which files can be read or written. `sandbox_mode` is `danger-full-access`: No filesystem sandboxing - all commands are permitted. Network access is {network_access}.
Filesystem sandboxing defines which files can be read or written. `sandbox_mode` is `danger-full-access`: No filesystem sandboxing - all commands are permitted. Network access is {{network_access}}.
@@ -1 +1 @@
Filesystem sandboxing defines which files can be read or written. `sandbox_mode` is `read-only`: The sandbox only permits reading files. Network access is {network_access}.
Filesystem sandboxing defines which files can be read or written. `sandbox_mode` is `read-only`: The sandbox only permits reading files. Network access is {{network_access}}.
@@ -1 +1 @@
Filesystem sandboxing defines which files can be read or written. `sandbox_mode` is `workspace-write`: The sandbox permits reading files, and editing files in `cwd` and `writable_roots`. Editing files in other directories requires approval. Network access is {network_access}.
Filesystem sandboxing defines which files can be read or written. `sandbox_mode` is `workspace-write`: The sandbox permits reading files, and editing files in `cwd` and `writable_roots`. Editing files in other directories requires approval. Network access is {{network_access}}.
+9
View File
@@ -512,6 +512,15 @@ pub enum Op {
ListModels,
}
impl From<Vec<UserInput>> for Op {
fn from(value: Vec<UserInput>) -> Self {
Op::UserInput {
items: value,
final_output_json_schema: None,
}
}
}
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, JsonSchema, TS)]
pub struct InterAgentCommunication {
pub author: AgentPath,
+1 -1
View File
@@ -20,7 +20,7 @@ fn system_bwrap_warning_for_lookup(system_bwrap_path: Option<PathBuf>) -> Option
pub fn find_system_bwrap_in_path() -> Option<PathBuf> {
let search_path = std::env::var_os("PATH")?;
let cwd = std::env::current_dir().ok()?;
find_system_bwrap_in_search_paths(std::iter::once(PathBuf::from(search_path)), &cwd)
find_system_bwrap_in_search_paths(std::env::split_paths(&search_path), &cwd)
}
fn find_system_bwrap_in_search_paths(
+6 -4
View File
@@ -33,7 +33,7 @@ exit 1
}
#[test]
fn finds_first_executable_bwrap_in_search_paths() {
fn finds_first_executable_bwrap_in_joined_search_path() {
let temp_dir = tempdir().expect("temp dir");
let cwd = temp_dir.path().join("cwd");
let first_dir = temp_dir.path().join("first");
@@ -43,15 +43,16 @@ fn finds_first_executable_bwrap_in_search_paths() {
std::fs::create_dir_all(&second_dir).expect("create second dir");
std::fs::write(first_dir.join("bwrap"), "not executable").expect("write non-executable bwrap");
let expected_bwrap = write_named_fake_bwrap_in(&second_dir);
let search_path = std::env::join_paths([first_dir, second_dir]).expect("join search path");
assert_eq!(
find_system_bwrap_in_search_paths(vec![first_dir, second_dir], &cwd),
find_system_bwrap_in_search_paths(std::env::split_paths(&search_path), &cwd),
Some(expected_bwrap)
);
}
#[test]
fn skips_workspace_local_bwrap_in_search_paths() {
fn skips_workspace_local_bwrap_in_joined_search_path() {
let temp_dir = tempdir().expect("temp dir");
let cwd = temp_dir.path().join("cwd");
let trusted_dir = temp_dir.path().join("trusted");
@@ -59,9 +60,10 @@ fn skips_workspace_local_bwrap_in_search_paths() {
std::fs::create_dir_all(&trusted_dir).expect("create trusted dir");
let _workspace_bwrap = write_named_fake_bwrap_in(&cwd);
let expected_bwrap = write_named_fake_bwrap_in(&trusted_dir);
let search_path = std::env::join_paths([cwd.clone(), trusted_dir]).expect("join search path");
assert_eq!(
find_system_bwrap_in_search_paths(vec![cwd.clone(), trusted_dir], &cwd),
find_system_bwrap_in_search_paths(std::env::split_paths(&search_path), &cwd),
Some(expected_bwrap)
);
}
+1
View File
@@ -8,6 +8,7 @@ version.workspace = true
workspace = true
[dependencies]
codex-protocol = { workspace = true }
rmcp = { workspace = true, default-features = false, features = [
"base64",
"macros",
+2 -1
View File
@@ -9,8 +9,9 @@ schema primitives that no longer need to live in `core/src/tools/spec.rs`:
- `JsonSchema`
- `AdditionalProperties`
- `ToolDefinition`
- `parse_tool_input_schema()`
- `ParsedMcpTool`
- `parse_dynamic_tool()`
- `parse_mcp_tool()`
- `mcp_call_tool_result_output_schema()`
+23
View File
@@ -0,0 +1,23 @@
use crate::ToolDefinition;
use crate::parse_tool_input_schema;
use codex_protocol::dynamic_tools::DynamicToolSpec;
pub fn parse_dynamic_tool(tool: &DynamicToolSpec) -> Result<ToolDefinition, serde_json::Error> {
let DynamicToolSpec {
name,
description,
input_schema,
defer_loading,
} = tool;
Ok(ToolDefinition {
name: name.clone(),
description: description.clone(),
input_schema: parse_tool_input_schema(input_schema)?,
output_schema: None,
defer_loading: *defer_loading,
})
}
#[cfg(test)]
#[path = "dynamic_tool_tests.rs"]
mod tests;
+70
View File
@@ -0,0 +1,70 @@
use super::parse_dynamic_tool;
use crate::JsonSchema;
use crate::ToolDefinition;
use codex_protocol::dynamic_tools::DynamicToolSpec;
use pretty_assertions::assert_eq;
use std::collections::BTreeMap;
#[test]
fn parse_dynamic_tool_sanitizes_input_schema() {
let tool = DynamicToolSpec {
name: "lookup_ticket".to_string(),
description: "Fetch a ticket".to_string(),
input_schema: serde_json::json!({
"properties": {
"id": {
"description": "Ticket identifier"
}
}
}),
defer_loading: false,
};
assert_eq!(
parse_dynamic_tool(&tool).expect("parse dynamic tool"),
ToolDefinition {
name: "lookup_ticket".to_string(),
description: "Fetch a ticket".to_string(),
input_schema: JsonSchema::Object {
properties: BTreeMap::from([(
"id".to_string(),
JsonSchema::String {
description: Some("Ticket identifier".to_string()),
},
)]),
required: None,
additional_properties: None,
},
output_schema: None,
defer_loading: false,
}
);
}
#[test]
fn parse_dynamic_tool_preserves_defer_loading() {
let tool = DynamicToolSpec {
name: "lookup_ticket".to_string(),
description: "Fetch a ticket".to_string(),
input_schema: serde_json::json!({
"type": "object",
"properties": {}
}),
defer_loading: true,
};
assert_eq!(
parse_dynamic_tool(&tool).expect("parse dynamic tool"),
ToolDefinition {
name: "lookup_ticket".to_string(),
description: "Fetch a ticket".to_string(),
input_schema: JsonSchema::Object {
properties: BTreeMap::new(),
required: None,
additional_properties: None,
},
output_schema: None,
defer_loading: true,
}
);
}
+4 -1
View File
@@ -1,11 +1,14 @@
//! Shared tool-schema parsing primitives that can live outside `codex-core`.
mod dynamic_tool;
mod json_schema;
mod mcp_tool;
mod tool_definition;
pub use dynamic_tool::parse_dynamic_tool;
pub use json_schema::AdditionalProperties;
pub use json_schema::JsonSchema;
pub use json_schema::parse_tool_input_schema;
pub use mcp_tool::ParsedMcpTool;
pub use mcp_tool::mcp_call_tool_result_output_schema;
pub use mcp_tool::parse_mcp_tool;
pub use tool_definition::ToolDefinition;
+8 -13
View File
@@ -1,18 +1,9 @@
use crate::JsonSchema;
use crate::ToolDefinition;
use crate::parse_tool_input_schema;
use serde_json::Value as JsonValue;
use serde_json::json;
/// Parsed MCP tool metadata and schemas that can be adapted into a higher-level
/// tool spec by downstream crates.
#[derive(Debug, PartialEq)]
pub struct ParsedMcpTool {
pub description: String,
pub input_schema: JsonSchema,
pub output_schema: JsonValue,
}
pub fn parse_mcp_tool(tool: &rmcp::model::Tool) -> Result<ParsedMcpTool, serde_json::Error> {
pub fn parse_mcp_tool(tool: &rmcp::model::Tool) -> Result<ToolDefinition, serde_json::Error> {
let mut serialized_input_schema = serde_json::Value::Object(tool.input_schema.as_ref().clone());
// OpenAI models mandate the "properties" field in the schema. Some MCP
@@ -34,10 +25,14 @@ pub fn parse_mcp_tool(tool: &rmcp::model::Tool) -> Result<ParsedMcpTool, serde_j
.map(|output_schema| serde_json::Value::Object(output_schema.as_ref().clone()))
.unwrap_or_else(|| JsonValue::Object(serde_json::Map::new()));
Ok(ParsedMcpTool {
Ok(ToolDefinition {
name: tool.name.to_string(),
description: tool.description.clone().map(Into::into).unwrap_or_default(),
input_schema,
output_schema: mcp_call_tool_result_output_schema(structured_content_schema),
output_schema: Some(mcp_call_tool_result_output_schema(
structured_content_schema,
)),
defer_loading: false,
})
}
+15 -9
View File
@@ -1,7 +1,7 @@
use super::ParsedMcpTool;
use super::mcp_call_tool_result_output_schema;
use super::parse_mcp_tool;
use crate::JsonSchema;
use crate::ToolDefinition;
use pretty_assertions::assert_eq;
use std::collections::BTreeMap;
@@ -31,14 +31,16 @@ fn parse_mcp_tool_inserts_empty_properties() {
assert_eq!(
parse_mcp_tool(&tool).expect("parse MCP tool"),
ParsedMcpTool {
ToolDefinition {
name: "no_props".to_string(),
description: "No properties".to_string(),
input_schema: JsonSchema::Object {
properties: BTreeMap::new(),
required: None,
additional_properties: None,
},
output_schema: mcp_call_tool_result_output_schema(serde_json::json!({})),
output_schema: Some(mcp_call_tool_result_output_schema(serde_json::json!({}))),
defer_loading: false,
}
);
}
@@ -67,14 +69,15 @@ fn parse_mcp_tool_preserves_top_level_output_schema() {
assert_eq!(
parse_mcp_tool(&tool).expect("parse MCP tool"),
ParsedMcpTool {
ToolDefinition {
name: "with_output".to_string(),
description: "Has output schema".to_string(),
input_schema: JsonSchema::Object {
properties: BTreeMap::new(),
required: None,
additional_properties: None,
},
output_schema: mcp_call_tool_result_output_schema(serde_json::json!({
output_schema: Some(mcp_call_tool_result_output_schema(serde_json::json!({
"properties": {
"result": {
"properties": {
@@ -83,7 +86,8 @@ fn parse_mcp_tool_preserves_top_level_output_schema() {
}
},
"required": ["result"]
})),
}))),
defer_loading: false,
}
);
}
@@ -105,16 +109,18 @@ fn parse_mcp_tool_preserves_output_schema_without_inferred_type() {
assert_eq!(
parse_mcp_tool(&tool).expect("parse MCP tool"),
ParsedMcpTool {
ToolDefinition {
name: "with_enum_output".to_string(),
description: "Has enum output schema".to_string(),
input_schema: JsonSchema::Object {
properties: BTreeMap::new(),
required: None,
additional_properties: None,
},
output_schema: mcp_call_tool_result_output_schema(serde_json::json!({
output_schema: Some(mcp_call_tool_result_output_schema(serde_json::json!({
"enum": ["ok", "error"]
})),
}))),
defer_loading: false,
}
);
}
+30
View File
@@ -0,0 +1,30 @@
use crate::JsonSchema;
use serde_json::Value as JsonValue;
/// Tool metadata and schemas that downstream crates can adapt into higher-level
/// tool specs.
#[derive(Debug, PartialEq)]
pub struct ToolDefinition {
pub name: String,
pub description: String,
pub input_schema: JsonSchema,
pub output_schema: Option<JsonValue>,
pub defer_loading: bool,
}
impl ToolDefinition {
pub fn renamed(mut self, name: String) -> Self {
self.name = name;
self
}
pub fn into_deferred(mut self) -> Self {
self.output_schema = None;
self.defer_loading = true;
self
}
}
#[cfg(test)]
#[path = "tool_definition_tests.rs"]
mod tests;
@@ -0,0 +1,43 @@
use super::ToolDefinition;
use crate::JsonSchema;
use pretty_assertions::assert_eq;
use std::collections::BTreeMap;
fn tool_definition() -> ToolDefinition {
ToolDefinition {
name: "lookup_order".to_string(),
description: "Look up an order".to_string(),
input_schema: JsonSchema::Object {
properties: BTreeMap::new(),
required: None,
additional_properties: None,
},
output_schema: Some(serde_json::json!({
"type": "object",
})),
defer_loading: false,
}
}
#[test]
fn renamed_overrides_name_only() {
assert_eq!(
tool_definition().renamed("mcp__orders__lookup_order".to_string()),
ToolDefinition {
name: "mcp__orders__lookup_order".to_string(),
..tool_definition()
}
);
}
#[test]
fn into_deferred_drops_output_schema_and_sets_defer_loading() {
assert_eq!(
tool_definition().into_deferred(),
ToolDefinition {
output_schema: None,
defer_loading: true,
..tool_definition()
}
);
}
+16 -88
View File
@@ -1,19 +1,9 @@
use codex_core::models_manager::collaboration_mode_presets::CollaborationModesConfig;
use codex_core::models_manager::collaboration_mode_presets::builtin_collaboration_mode_presets;
use codex_protocol::config_types::CollaborationModeMask;
use codex_protocol::config_types::ModeKind;
use codex_protocol::config_types::TUI_VISIBLE_COLLABORATION_MODES;
use codex_protocol::openai_models::ModelPreset;
use codex_protocol::openai_models::ReasoningEffort;
use std::convert::Infallible;
const COLLABORATION_MODE_PLAN: &str =
include_str!("../../core/templates/collaboration_mode/plan.md");
const COLLABORATION_MODE_DEFAULT: &str =
include_str!("../../core/templates/collaboration_mode/default.md");
const KNOWN_MODE_NAMES_PLACEHOLDER: &str = "{{KNOWN_MODE_NAMES}}";
const REQUEST_USER_INPUT_AVAILABILITY_PLACEHOLDER: &str = "{{REQUEST_USER_INPUT_AVAILABILITY}}";
const ASKING_QUESTIONS_GUIDANCE_PLACEHOLDER: &str = "{{ASKING_QUESTIONS_GUIDANCE}}";
#[derive(Debug, Clone)]
pub(crate) struct ModelCatalog {
models: Vec<ModelPreset>,
@@ -40,83 +30,21 @@ impl ModelCatalog {
}
}
fn builtin_collaboration_mode_presets(
collaboration_modes_config: CollaborationModesConfig,
) -> Vec<CollaborationModeMask> {
vec![plan_preset(), default_preset(collaboration_modes_config)]
}
#[cfg(test)]
mod tests {
use super::*;
use pretty_assertions::assert_eq;
fn plan_preset() -> CollaborationModeMask {
CollaborationModeMask {
name: ModeKind::Plan.display_name().to_string(),
mode: Some(ModeKind::Plan),
model: None,
reasoning_effort: Some(Some(ReasoningEffort::Medium)),
developer_instructions: Some(Some(COLLABORATION_MODE_PLAN.to_string())),
}
}
fn default_preset(collaboration_modes_config: CollaborationModesConfig) -> CollaborationModeMask {
CollaborationModeMask {
name: ModeKind::Default.display_name().to_string(),
mode: Some(ModeKind::Default),
model: None,
reasoning_effort: None,
developer_instructions: Some(Some(default_mode_instructions(collaboration_modes_config))),
}
}
fn default_mode_instructions(collaboration_modes_config: CollaborationModesConfig) -> String {
let known_mode_names = format_mode_names(&TUI_VISIBLE_COLLABORATION_MODES);
let request_user_input_availability = request_user_input_availability_message(
ModeKind::Default,
collaboration_modes_config.default_mode_request_user_input,
);
let asking_questions_guidance = asking_questions_guidance_message(
collaboration_modes_config.default_mode_request_user_input,
);
COLLABORATION_MODE_DEFAULT
.replace(KNOWN_MODE_NAMES_PLACEHOLDER, &known_mode_names)
.replace(
REQUEST_USER_INPUT_AVAILABILITY_PLACEHOLDER,
&request_user_input_availability,
)
.replace(
ASKING_QUESTIONS_GUIDANCE_PLACEHOLDER,
&asking_questions_guidance,
)
}
fn format_mode_names(modes: &[ModeKind]) -> String {
let mode_names: Vec<&str> = modes.iter().map(|mode| mode.display_name()).collect();
match mode_names.as_slice() {
[] => "none".to_string(),
[mode_name] => (*mode_name).to_string(),
[first, second] => format!("{first} and {second}"),
[..] => mode_names.join(", "),
}
}
fn request_user_input_availability_message(
mode: ModeKind,
default_mode_request_user_input: bool,
) -> String {
let mode_name = mode.display_name();
if mode.allows_request_user_input()
|| (default_mode_request_user_input && mode == ModeKind::Default)
{
format!("The `request_user_input` tool is available in {mode_name} mode.")
} else {
format!(
"The `request_user_input` tool is unavailable in {mode_name} mode. If you call it while in {mode_name} mode, it will return an error."
)
}
}
fn asking_questions_guidance_message(default_mode_request_user_input: bool) -> String {
if default_mode_request_user_input {
"In Default mode, strongly prefer making reasonable assumptions and executing the user's request rather than stopping to ask questions. If you absolutely must ask a question because the answer cannot be discovered from local context and a reasonable assumption would be risky, prefer using the `request_user_input` tool rather than writing a multiple choice question as a textual assistant message. Never write a multiple choice question as a textual assistant message.".to_string()
} else {
"In Default mode, strongly prefer making reasonable assumptions and executing the user's request rather than stopping to ask questions. If you absolutely must ask a question because the answer cannot be discovered from local context and a reasonable assumption would be risky, ask the user directly with a concise plain-text question. Never write a multiple choice question as a textual assistant message.".to_string()
#[test]
fn list_collaboration_modes_matches_core_presets() {
let collaboration_modes_config = CollaborationModesConfig {
default_mode_request_user_input: true,
};
let catalog = ModelCatalog::new(Vec::new(), collaboration_modes_config);
assert_eq!(
catalog.list_collaboration_modes(),
builtin_collaboration_mode_presets(collaboration_modes_config)
);
}
}