Merge 05535a9d5a into sapling-pr-archive-bolinfest

This commit is contained in:
Michael Bolin
2026-04-23 10:00:15 -07:00
committed by GitHub
53 changed files with 1422 additions and 433 deletions
@@ -325,7 +325,7 @@ impl From<CoreAskForApproval> for AskForApproval {
pub enum ApprovalsReviewer {
#[serde(rename = "user")]
User,
#[serde(rename = "auto_review", alias = "guardian_subagent")]
#[serde(rename = "guardian_subagent", alias = "auto_review")]
AutoReview,
}
@@ -7500,7 +7500,7 @@ mod tests {
);
assert_eq!(
serde_json::to_string(&ApprovalsReviewer::AutoReview).expect("serialize reviewer"),
"\"auto_review\""
"\"guardian_subagent\""
);
for value in ["user", "auto_review", "guardian_subagent"] {
@@ -72,6 +72,7 @@ use wiremock::matchers::path;
use wiremock::matchers::path_regex;
const DEFAULT_TIMEOUT: Duration = Duration::from_secs(10);
const DELEGATED_SHELL_TOOL_TIMEOUT_MS: u64 = 30_000;
const STARTUP_CONTEXT_HEADER: &str = "Startup context from Codex.";
const V2_STEERING_ACKNOWLEDGEMENT: &str =
"This was sent to steer the previous background agent task.";
@@ -1781,7 +1782,9 @@ async fn webrtc_v2_tool_call_delegated_turn_can_execute_shell_tool() -> Result<(
create_shell_command_sse_response(
realtime_tool_ok_command(),
/*workdir*/ None,
Some(5000),
// Windows CI can spend several seconds starting the nested PowerShell command. This
// test verifies delegated shell-tool plumbing, not timeout enforcement.
Some(DELEGATED_SHELL_TOOL_TIMEOUT_MS),
"shell_call",
)?,
create_final_assistant_message_sse_response("shell tool finished")?,
+25
View File
@@ -176,6 +176,7 @@ enum Subcommand {
}
#[derive(Debug, Parser)]
#[command(bin_name = "codex plugin")]
struct PluginCli {
#[clap(flatten)]
pub config_overrides: CliConfigOverrides,
@@ -1773,6 +1774,30 @@ mod tests {
assert!(matches!(cli.subcommand, Some(Subcommand::Responses(_))));
}
fn help_from_args(args: &[&str]) -> String {
let err = MultitoolCli::try_parse_from(args).expect_err("help should short-circuit");
assert_eq!(err.kind(), clap::error::ErrorKind::DisplayHelp);
err.to_string()
}
#[test]
fn plugin_marketplace_help_uses_plugin_namespace() {
let help = help_from_args(&["codex", "plugin", "marketplace", "--help"]);
assert!(
help.contains("Usage: codex plugin marketplace [OPTIONS] <COMMAND>"),
"{help}"
);
for (subcommand, usage) in [
("add", "Usage: codex plugin marketplace add"),
("upgrade", "Usage: codex plugin marketplace upgrade"),
("remove", "Usage: codex plugin marketplace remove"),
] {
let help = help_from_args(&["codex", "plugin", "marketplace", subcommand, "--help"]);
assert!(help.contains(usage), "{help}");
}
}
#[test]
fn plugin_marketplace_add_parses_under_plugin() {
let cli =
+4
View File
@@ -13,6 +13,7 @@ use codex_core::plugins::remove_marketplace;
use codex_utils_cli::CliConfigOverrides;
#[derive(Debug, Parser)]
#[command(bin_name = "codex plugin marketplace")]
pub struct MarketplaceCli {
#[clap(flatten)]
pub config_overrides: CliConfigOverrides,
@@ -29,6 +30,7 @@ enum MarketplaceSubcommand {
}
#[derive(Debug, Parser)]
#[command(bin_name = "codex plugin marketplace add")]
struct AddMarketplaceArgs {
/// Marketplace source. Supports owner/repo[@ref], HTTP(S) Git URLs, SSH URLs,
/// or local marketplace root directories.
@@ -46,11 +48,13 @@ struct AddMarketplaceArgs {
}
#[derive(Debug, Parser)]
#[command(bin_name = "codex plugin marketplace upgrade")]
struct UpgradeMarketplaceArgs {
marketplace_name: Option<String>,
}
#[derive(Debug, Parser)]
#[command(bin_name = "codex plugin marketplace remove")]
struct RemoveMarketplaceArgs {
/// Configured marketplace name to remove.
marketplace_name: String,
+22
View File
@@ -6610,6 +6610,28 @@ async fn feature_requirements_normalize_effective_feature_values() -> std::io::R
Ok(())
}
#[tokio::test]
async fn feature_requirements_auto_review_disables_guardian_approval() -> std::io::Result<()> {
let codex_home = TempDir::new()?;
let config = ConfigBuilder::without_managed_config_for_tests()
.codex_home(codex_home.path().to_path_buf())
.cloud_requirements(CloudRequirementsLoader::new(async {
Ok(Some(crate::config_loader::ConfigRequirementsToml {
feature_requirements: Some(crate::config_loader::FeatureRequirementsToml {
entries: BTreeMap::from([("auto_review".to_string(), false)]),
}),
..Default::default()
}))
}))
.build()
.await?;
assert!(!config.features.enabled(Feature::GuardianApproval));
Ok(())
}
#[tokio::test]
async fn browser_feature_requirements_are_valid() -> std::io::Result<()> {
let codex_home = TempDir::new()?;
@@ -199,6 +199,11 @@ fn parse_feature_requirements(
) -> BTreeMap<Feature, bool> {
let mut pinned_features = BTreeMap::new();
for (key, enabled) in feature_requirements.entries {
if key == "auto_review" {
pinned_features.insert(Feature::GuardianApproval, enabled);
continue;
}
if let Some(feature) = canonical_feature_for_key(&key) {
pinned_features.insert(feature, enabled);
continue;
+7
View File
@@ -1948,6 +1948,13 @@ impl Config {
let history = cfg.history.unwrap_or_default();
let agent_max_threads_from_config = cfg.agents.as_ref().and_then(|agents| agents.max_threads);
if features.enabled(Feature::MultiAgentV2) && agent_max_threads_from_config.is_some() {
return Err(std::io::Error::new(
std::io::ErrorKind::InvalidInput,
"agents.max_threads cannot be set when multi_agent_v2 is enabled",
));
}
let agent_max_threads = cfg
.agents
.as_ref()
-2
View File
@@ -20,7 +20,6 @@ mod realtime_end_instructions;
mod realtime_start_instructions;
mod realtime_start_with_instructions;
mod skill_instructions;
mod spawn_agent_instructions;
mod subagent_notification;
mod turn_aborted;
mod user_instructions;
@@ -50,7 +49,6 @@ pub(crate) use realtime_end_instructions::RealtimeEndInstructions;
pub(crate) use realtime_start_instructions::RealtimeStartInstructions;
pub(crate) use realtime_start_with_instructions::RealtimeStartWithInstructions;
pub(crate) use skill_instructions::SkillInstructions;
pub(crate) use spawn_agent_instructions::SpawnAgentInstructions;
pub(crate) use subagent_notification::SubagentNotification;
pub(crate) use turn_aborted::TurnAborted;
pub(crate) use user_instructions::UserInstructions;
@@ -1,14 +0,0 @@
use super::ContextualUserFragment;
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub(crate) struct SpawnAgentInstructions;
impl ContextualUserFragment for SpawnAgentInstructions {
const ROLE: &'static str = "developer";
const START_MARKER: &'static str = "<spawned_agent_context>";
const END_MARKER: &'static str = "</spawned_agent_context>";
fn body(&self) -> String {
"\nYou are a newly spawned agent in a team of agents collaborating to complete a task. You can spawn sub-agents to handle subtasks, and those sub-agents can spawn their own sub-agents. You are responsible for returning the response to your assigned task in the final channel. When you give your response, the contents of your response in the final channel will be immediately delivered back to your parent agent. The prior conversation history was forked from your parent agent. Treat the next user message as your assigned task, and use the forked history only as background context.\n".to_string()
}
}
+26 -12
View File
@@ -35,6 +35,7 @@ use crate::context::HookAdditionalContext;
use crate::event_mapping::parse_turn_item;
use crate::session::session::Session;
use crate::session::turn_context::TurnContext;
use crate::tools::hook_names::HookToolName;
use crate::tools::sandboxing::PermissionRequestPayload;
pub(crate) struct HookRuntimeOutcome {
@@ -137,9 +138,8 @@ pub(crate) async fn run_pre_tool_use_hooks(
sess: &Arc<Session>,
turn_context: &Arc<TurnContext>,
tool_use_id: String,
tool_name: String,
matcher_aliases: Vec<String>,
command: String,
tool_name: &HookToolName,
tool_input: &Value,
) -> Option<String> {
let request = PreToolUseRequest {
session_id: sess.conversation_id,
@@ -148,10 +148,10 @@ pub(crate) async fn run_pre_tool_use_hooks(
transcript_path: sess.hook_transcript_path().await,
model: turn_context.model_info.slug.clone(),
permission_mode: hook_permission_mode(turn_context),
tool_name,
matcher_aliases,
tool_name: tool_name.name().to_string(),
matcher_aliases: tool_name.matcher_aliases().to_vec(),
tool_use_id,
command,
tool_input: tool_input.clone(),
};
let preview_runs = sess.hooks().preview_pre_tool_use(&request);
emit_hook_started_events(sess, turn_context, preview_runs).await;
@@ -163,7 +163,22 @@ pub(crate) async fn run_pre_tool_use_hooks(
} = sess.hooks().run_pre_tool_use(request).await;
emit_hook_completed_events(sess, turn_context, hook_events).await;
if should_block { block_reason } else { None }
if should_block {
block_reason.map(|reason| {
if (tool_name.name() == "Bash" || tool_name.name() == "apply_patch")
&& let Some(command) = tool_input.get("command").and_then(Value::as_str)
{
format!("Command blocked by PreToolUse hook: {reason}. Command: {command}")
} else {
format!(
"Tool call blocked by PreToolUse hook: {reason}. Tool: {}",
tool_name.name()
)
}
})
} else {
None
}
}
// PermissionRequest hooks share the same preview/start/completed event flow as
@@ -185,8 +200,7 @@ pub(crate) async fn run_permission_request_hooks(
tool_name: payload.tool_name.name().to_string(),
matcher_aliases: payload.tool_name.matcher_aliases().to_vec(),
run_id_suffix: run_id_suffix.to_string(),
command: payload.command,
description: payload.description,
tool_input: payload.tool_input,
};
let preview_runs = sess.hooks().preview_permission_request(&request);
emit_hook_started_events(sess, turn_context, preview_runs).await;
@@ -202,7 +216,7 @@ pub(crate) async fn run_permission_request_hooks(
/// Runs matching `PostToolUse` hooks after a tool has produced a successful output.
///
/// The `tool_name`, matcher aliases, `command`, and `tool_response` values are
/// The `tool_name`, matcher aliases, `tool_input`, and `tool_response` values are
/// already adapted by the tool handler into the stable hook contract. Passing
/// raw internal tool data here would leak implementation details into user hook
/// matchers and hook logs.
@@ -212,7 +226,7 @@ pub(crate) async fn run_post_tool_use_hooks(
tool_use_id: String,
tool_name: String,
matcher_aliases: Vec<String>,
command: String,
tool_input: Value,
tool_response: Value,
) -> PostToolUseOutcome {
let request = PostToolUseRequest {
@@ -225,7 +239,7 @@ pub(crate) async fn run_post_tool_use_hooks(
tool_name,
matcher_aliases,
tool_use_id,
command,
tool_input,
tool_response,
};
let preview_runs = sess.hooks().preview_post_tool_use(&request);
+157 -130
View File
@@ -25,16 +25,20 @@ use crate::guardian::guardian_timeout_message;
use crate::guardian::new_guardian_review_id;
use crate::guardian::review_approval_request;
use crate::guardian::routes_approval_to_guardian;
use crate::hook_runtime::run_permission_request_hooks;
use crate::mcp_openai_file::rewrite_mcp_tool_arguments_for_openai_files;
use crate::mcp_tool_approval_templates::RenderedMcpToolApprovalParam;
use crate::mcp_tool_approval_templates::render_mcp_tool_approval_template;
use crate::session::session::Session;
use crate::session::turn_context::TurnContext;
use crate::tools::hook_names::HookToolName;
use crate::tools::sandboxing::PermissionRequestPayload;
use codex_analytics::AppInvocation;
use codex_analytics::InvocationType;
use codex_analytics::build_track_events_context;
use codex_config::types::AppToolApproval;
use codex_features::Feature;
use codex_hooks::PermissionRequestDecision;
use codex_mcp::CODEX_APPS_MCP_SERVER_NAME;
use codex_mcp::SandboxState;
use codex_mcp::declared_openai_file_input_param_names;
@@ -59,6 +63,7 @@ use codex_utils_absolute_path::AbsolutePathBuf;
use rmcp::model::ToolAnnotations;
use serde::Deserialize;
use serde::Serialize;
use serde_json::Value as JsonValue;
use std::sync::Arc;
use toml_edit::value;
use tracing::Instrument;
@@ -77,8 +82,9 @@ pub(crate) async fn handle_mcp_tool_call(
call_id: String,
server: String,
tool_name: String,
hook_tool_name: String,
arguments: String,
) -> CallToolResult {
) -> HandledMcpToolCall {
// Parse the `arguments` as JSON. An empty string is OK, but invalid JSON
// is not.
let arguments_value = if arguments.trim().is_empty() {
@@ -88,7 +94,10 @@ pub(crate) async fn handle_mcp_tool_call(
Ok(value) => Some(value),
Err(e) => {
error!("failed to parse tool call arguments: {e}");
return CallToolResult::from_error_text(format!("err: {e}"));
return HandledMcpToolCall {
result: CallToolResult::from_error_text(format!("err: {e}")),
tool_input: JsonValue::Object(serde_json::Map::new()),
};
}
}
};
@@ -144,7 +153,11 @@ pub(crate) async fn handle_mcp_tool_call(
/*inc*/ 1,
&[("status", status)],
);
return CallToolResult::from_result(result);
return HandledMcpToolCall {
result: CallToolResult::from_result(result),
tool_input: arguments_value
.unwrap_or_else(|| JsonValue::Object(serde_json::Map::new())),
};
}
let request_meta =
build_mcp_tool_call_request_meta(turn_context.as_ref(), &server, metadata.as_ref());
@@ -154,13 +167,6 @@ pub(crate) async fn handle_mcp_tool_call(
let connector_name = metadata
.as_ref()
.and_then(|metadata| metadata.connector_name.clone());
let server_origin = sess
.services
.mcp_connection_manager
.read()
.await
.server_origin(&server)
.map(str::to_string);
let tool_call_begin_event = EventMsg::McpToolCallBegin(McpToolCallBeginEvent {
call_id: call_id.clone(),
@@ -174,115 +180,64 @@ pub(crate) async fn handle_mcp_tool_call(
turn_context,
&call_id,
&invocation,
&hook_tool_name,
metadata.as_ref(),
approval_mode,
)
.await
{
let (result, call_duration) = match decision {
let result = match decision {
McpToolApprovalDecision::Accept
| McpToolApprovalDecision::AcceptForSession
| McpToolApprovalDecision::AcceptAndRemember => {
maybe_mark_thread_memory_mode_polluted(sess.as_ref(), turn_context.as_ref()).await;
let start = Instant::now();
let result = async {
execute_mcp_tool_call(
sess.as_ref(),
turn_context.as_ref(),
&server,
&tool_name,
arguments_value.clone(),
metadata.as_ref(),
request_meta.clone(),
)
.await
}
.instrument(mcp_tool_call_span(
return handle_approved_mcp_tool_call(
sess.as_ref(),
turn_context.as_ref(),
McpToolCallSpanFields {
server_name: &server,
tool_name: &tool_name,
call_id: &call_id,
server_origin: server_origin.as_deref(),
connector_id: connector_id.as_deref(),
connector_name: connector_name.as_deref(),
},
))
.await;
if let Err(error) = &result {
tracing::warn!("MCP tool call error: {error:?}");
}
let duration = start.elapsed();
let tool_call_end_event = EventMsg::McpToolCallEnd(McpToolCallEndEvent {
call_id: call_id.clone(),
&call_id,
invocation,
mcp_app_resource_uri: mcp_app_resource_uri.clone(),
duration,
result: result.clone(),
});
notify_mcp_tool_call_event(
sess.as_ref(),
turn_context.as_ref(),
tool_call_end_event.clone(),
metadata.as_ref(),
request_meta,
mcp_app_resource_uri,
)
.await;
maybe_track_codex_app_used(
sess.as_ref(),
turn_context.as_ref(),
&server,
&tool_name,
)
.await;
(result, Some(duration))
}
McpToolApprovalDecision::Decline { message } => {
let message = message.unwrap_or_else(|| "user rejected MCP tool call".to_string());
(
notify_mcp_tool_call_skip(
sess.as_ref(),
turn_context.as_ref(),
&call_id,
invocation,
mcp_app_resource_uri.clone(),
message,
/*already_started*/ true,
)
.await,
None,
notify_mcp_tool_call_skip(
sess.as_ref(),
turn_context.as_ref(),
&call_id,
invocation,
mcp_app_resource_uri.clone(),
message,
/*already_started*/ true,
)
.await
}
McpToolApprovalDecision::Cancel => {
let message = "user cancelled MCP tool call".to_string();
(
notify_mcp_tool_call_skip(
sess.as_ref(),
turn_context.as_ref(),
&call_id,
invocation,
mcp_app_resource_uri.clone(),
message,
/*already_started*/ true,
)
.await,
None,
notify_mcp_tool_call_skip(
sess.as_ref(),
turn_context.as_ref(),
&call_id,
invocation,
mcp_app_resource_uri.clone(),
message,
/*already_started*/ true,
)
.await
}
McpToolApprovalDecision::BlockedBySafetyMonitor(message) => {
(
notify_mcp_tool_call_skip(
sess.as_ref(),
turn_context.as_ref(),
&call_id,
invocation,
mcp_app_resource_uri.clone(),
message,
/*already_started*/ true,
)
.await,
None,
notify_mcp_tool_call_skip(
sess.as_ref(),
turn_context.as_ref(),
&call_id,
invocation,
mcp_app_resource_uri.clone(),
message,
/*already_started*/ true,
)
.await
}
};
@@ -293,37 +248,93 @@ pub(crate) async fn handle_mcp_tool_call(
&tool_name,
connector_id.as_deref(),
connector_name.as_deref(),
call_duration,
/*duration*/ None,
);
return CallToolResult::from_result(result);
return HandledMcpToolCall {
result: CallToolResult::from_result(result),
tool_input: arguments_value
.unwrap_or_else(|| JsonValue::Object(serde_json::Map::new())),
};
}
maybe_mark_thread_memory_mode_polluted(sess.as_ref(), turn_context.as_ref()).await;
handle_approved_mcp_tool_call(
sess.as_ref(),
turn_context.as_ref(),
&call_id,
invocation,
metadata.as_ref(),
request_meta,
mcp_app_resource_uri,
)
.await
}
pub(crate) struct HandledMcpToolCall {
pub(crate) result: CallToolResult,
pub(crate) tool_input: JsonValue,
}
async fn handle_approved_mcp_tool_call(
sess: &Session,
turn_context: &TurnContext,
call_id: &str,
invocation: McpInvocation,
metadata: Option<&McpToolApprovalMetadata>,
request_meta: Option<JsonValue>,
mcp_app_resource_uri: Option<String>,
) -> HandledMcpToolCall {
maybe_mark_thread_memory_mode_polluted(sess, turn_context).await;
let server = invocation.server.clone();
let tool_name = invocation.tool.clone();
let arguments_value = invocation.arguments.clone();
let connector_id = metadata.and_then(|metadata| metadata.connector_id.as_deref());
let connector_name = metadata.and_then(|metadata| metadata.connector_name.as_deref());
let server_origin = sess
.services
.mcp_connection_manager
.read()
.await
.server_origin(&server)
.map(str::to_string);
let start = Instant::now();
let rewrite = rewrite_mcp_tool_arguments_for_openai_files(
sess,
turn_context,
arguments_value.clone(),
metadata.and_then(|metadata| metadata.openai_file_input_params.as_deref()),
)
.await;
let tool_input = match &rewrite {
Ok(Some(rewritten_arguments)) => rewritten_arguments.clone(),
Ok(None) | Err(_) => arguments_value
.clone()
.unwrap_or_else(|| JsonValue::Object(serde_json::Map::new())),
};
let result = async {
let rewritten_arguments = rewrite?;
execute_mcp_tool_call(
sess.as_ref(),
turn_context.as_ref(),
sess,
turn_context,
&server,
&tool_name,
arguments_value.clone(),
metadata.as_ref(),
rewritten_arguments,
request_meta,
)
.await
}
.instrument(mcp_tool_call_span(
sess.as_ref(),
turn_context.as_ref(),
sess,
turn_context,
McpToolCallSpanFields {
server_name: &server,
tool_name: &tool_name,
call_id: &call_id,
call_id,
server_origin: server_origin.as_deref(),
connector_id: connector_id.as_deref(),
connector_name: connector_name.as_deref(),
connector_id,
connector_name,
},
))
.await;
@@ -332,32 +343,29 @@ pub(crate) async fn handle_mcp_tool_call(
}
let duration = start.elapsed();
let tool_call_end_event = EventMsg::McpToolCallEnd(McpToolCallEndEvent {
call_id: call_id.clone(),
call_id: call_id.to_string(),
invocation,
mcp_app_resource_uri,
duration,
result: result.clone(),
});
notify_mcp_tool_call_event(
sess.as_ref(),
turn_context.as_ref(),
tool_call_end_event.clone(),
)
.await;
maybe_track_codex_app_used(sess.as_ref(), turn_context.as_ref(), &server, &tool_name).await;
notify_mcp_tool_call_event(sess, turn_context, tool_call_end_event.clone()).await;
maybe_track_codex_app_used(sess, turn_context, &server, &tool_name).await;
let status = if result.is_ok() { "ok" } else { "error" };
emit_mcp_call_metrics(
turn_context.as_ref(),
turn_context,
status,
&tool_name,
connector_id.as_deref(),
connector_name.as_deref(),
connector_id,
connector_name,
Some(duration),
);
CallToolResult::from_result(result)
HandledMcpToolCall {
result: CallToolResult::from_result(result),
tool_input,
}
}
fn emit_mcp_call_metrics(
@@ -466,17 +474,9 @@ async fn execute_mcp_tool_call(
turn_context: &TurnContext,
server: &str,
tool_name: &str,
arguments_value: Option<serde_json::Value>,
metadata: Option<&McpToolApprovalMetadata>,
request_meta: Option<serde_json::Value>,
rewritten_arguments: Option<JsonValue>,
request_meta: Option<JsonValue>,
) -> Result<CallToolResult, String> {
let rewritten_arguments = rewrite_mcp_tool_arguments_for_openai_files(
sess,
turn_context,
arguments_value,
metadata.and_then(|metadata| metadata.openai_file_input_params.as_deref()),
)
.await?;
let request_meta =
with_mcp_tool_call_thread_id_meta(request_meta, &sess.conversation_id.to_string());
let request_meta =
@@ -815,6 +815,7 @@ async fn maybe_request_mcp_tool_approval(
turn_context: &Arc<TurnContext>,
call_id: &str,
invocation: &McpInvocation,
hook_tool_name: &str,
metadata: Option<&McpToolApprovalMetadata>,
approval_mode: AppToolApproval,
) -> Option<McpToolApprovalDecision> {
@@ -864,6 +865,32 @@ async fn maybe_request_mcp_tool_approval(
{
return Some(McpToolApprovalDecision::Accept);
}
match run_permission_request_hooks(
sess,
turn_context,
call_id,
PermissionRequestPayload {
tool_name: HookToolName::new(hook_tool_name),
tool_input: invocation
.arguments
.clone()
.unwrap_or_else(|| serde_json::Value::Object(serde_json::Map::new())),
},
)
.await
{
Some(PermissionRequestDecision::Allow) => {
return Some(McpToolApprovalDecision::Accept);
}
Some(PermissionRequestDecision::Deny { message }) => {
return Some(McpToolApprovalDecision::Decline {
message: Some(message),
});
}
None => {}
}
let tool_call_mcp_elicitation_enabled = turn_context
.config
.features
+283
View File
@@ -12,6 +12,8 @@ use codex_config::types::ApprovalsReviewer;
use codex_config::types::AppsConfigToml;
use codex_config::types::McpServerConfig;
use codex_config::types::McpServerToolConfig;
use codex_hooks::Hooks;
use codex_hooks::HooksConfig;
use codex_model_provider::create_model_provider;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::SandboxPolicy;
@@ -76,6 +78,81 @@ fn prompt_options(
}
}
fn install_mcp_permission_request_hook(
session: &mut Session,
turn_context: &TurnContext,
matcher: &str,
hook_output: &serde_json::Value,
) -> std::path::PathBuf {
let script_path = turn_context
.config
.codex_home
.join("mcp_permission_request_hook.py");
let log_path = turn_context
.config
.codex_home
.join("mcp_permission_request_hook_log.jsonl");
let hook_output = hook_output.to_string();
std::fs::create_dir_all(&turn_context.config.codex_home)
.expect("create codex home for MCP permission hook");
let script = format!(
r#"import json
from pathlib import Path
import sys
payload = json.load(sys.stdin)
with Path(r"{log_path}").open("a", encoding="utf-8") as handle:
handle.write(json.dumps(payload) + "\n")
print({hook_output:?})
"#,
log_path = log_path.display(),
hook_output = hook_output,
);
std::fs::write(&script_path, script).expect("write MCP permission hook script");
let python = if cfg!(windows) { "python" } else { "python3" };
let script_path_arg = if cfg!(windows) {
script_path.display().to_string()
} else {
format!(
"'{}'",
script_path.display().to_string().replace('\'', "'\\''")
)
};
std::fs::write(
turn_context.config.codex_home.join("hooks.json"),
serde_json::json!({
"hooks": {
"PermissionRequest": [{
"matcher": matcher,
"hooks": [{
"type": "command",
"command": format!("{python} {script_path_arg}"),
"timeout_sec": 5,
}]
}]
}
})
.to_string(),
)
.expect("write hooks.json");
session.services.hooks = Hooks::new(HooksConfig {
feature_enabled: true,
config_layer_stack: Some(turn_context.config.config_layer_stack.clone()),
shell_program: (!cfg!(windows)).then_some("/bin/sh".to_string()),
shell_args: if cfg!(windows) {
Vec::new()
} else {
vec!["-c".to_string()]
},
..HooksConfig::default()
});
log_path.to_path_buf()
}
#[test]
fn mcp_app_resource_uri_reads_known_tool_meta_keys() {
let nested = serde_json::json!({
@@ -1381,6 +1458,7 @@ async fn approve_mode_skips_when_annotations_do_not_require_approval() {
&turn_context,
"call-1",
&invocation,
"mcp__test__tool",
Some(&metadata),
AppToolApproval::Approve,
)
@@ -1453,6 +1531,7 @@ async fn guardian_mode_skips_auto_when_annotations_do_not_require_approval() {
&turn_context,
"call-guardian",
&invocation,
"mcp__test__tool",
Some(&metadata),
AppToolApproval::Auto,
)
@@ -1461,6 +1540,203 @@ async fn guardian_mode_skips_auto_when_annotations_do_not_require_approval() {
assert_eq!(decision, None);
}
#[tokio::test]
async fn permission_request_hook_allows_mcp_tool_call() {
let (mut session, turn_context) = make_session_and_context().await;
let log_path = install_mcp_permission_request_hook(
&mut session,
&turn_context,
"mcp__memory__.*",
&serde_json::json!({
"hookSpecificOutput": {
"hookEventName": "PermissionRequest",
"decision": { "behavior": "allow" }
}
}),
);
let session = Arc::new(session);
let turn_context = Arc::new(turn_context);
let invocation = McpInvocation {
server: "memory".to_string(),
tool: "create_entities".to_string(),
arguments: Some(serde_json::json!({
"entities": [{
"name": "Ada",
"entityType": "person"
}]
})),
};
let metadata = McpToolApprovalMetadata {
annotations: Some(annotations(
Some(false),
Some(true),
/*open_world*/ None,
)),
connector_id: None,
connector_name: None,
connector_description: None,
tool_title: Some("Create entities".to_string()),
tool_description: None,
mcp_app_resource_uri: None,
codex_apps_meta: None,
openai_file_input_params: None,
};
let decision = maybe_request_mcp_tool_approval(
&session,
&turn_context,
"call-mcp-hook",
&invocation,
"mcp__memory__create_entities",
Some(&metadata),
AppToolApproval::Auto,
)
.await;
assert_eq!(decision, Some(McpToolApprovalDecision::Accept));
let log = std::fs::read_to_string(log_path).expect("read MCP permission hook log");
let inputs = log
.lines()
.map(|line| serde_json::from_str::<serde_json::Value>(line).expect("parse hook input"))
.collect::<Vec<_>>();
assert_eq!(
inputs,
vec![serde_json::json!({
"session_id": session.conversation_id,
"turn_id": "turn_id",
"cwd": turn_context.cwd,
"transcript_path": null,
"model": turn_context.model_info.slug,
"permission_mode": "default",
"tool_name": "mcp__memory__create_entities",
"hook_event_name": "PermissionRequest",
"tool_input": {
"entities": [{
"name": "Ada",
"entityType": "person"
}]
}
})]
);
}
#[tokio::test]
async fn permission_request_hook_uses_hook_tool_name_without_metadata() {
let (mut session, turn_context) = make_session_and_context().await;
let log_path = install_mcp_permission_request_hook(
&mut session,
&turn_context,
"mcp__memory__.*",
&serde_json::json!({
"hookSpecificOutput": {
"hookEventName": "PermissionRequest",
"decision": { "behavior": "allow" }
}
}),
);
let session = Arc::new(session);
let turn_context = Arc::new(turn_context);
let invocation = McpInvocation {
server: "memory".to_string(),
tool: "create_entities".to_string(),
arguments: Some(serde_json::json!({ "entities": [] })),
};
let decision = maybe_request_mcp_tool_approval(
&session,
&turn_context,
"call-mcp-hook-no-metadata",
&invocation,
"mcp__memory__create_entities",
/*metadata*/ None,
AppToolApproval::Auto,
)
.await;
assert_eq!(decision, Some(McpToolApprovalDecision::Accept));
let log = std::fs::read_to_string(log_path).expect("read MCP permission hook log");
let inputs = log
.lines()
.map(|line| serde_json::from_str::<serde_json::Value>(line).expect("parse hook input"))
.collect::<Vec<_>>();
assert_eq!(
inputs,
vec![serde_json::json!({
"session_id": session.conversation_id,
"turn_id": "turn_id",
"cwd": turn_context.cwd,
"transcript_path": null,
"model": turn_context.model_info.slug,
"permission_mode": "default",
"tool_name": "mcp__memory__create_entities",
"hook_event_name": "PermissionRequest",
"tool_input": { "entities": [] }
})]
);
}
#[tokio::test]
async fn permission_request_hook_runs_after_remembered_mcp_approval() {
let (mut session, turn_context) = make_session_and_context().await;
let log_path = install_mcp_permission_request_hook(
&mut session,
&turn_context,
"mcp__memory__.*",
&serde_json::json!({
"hookSpecificOutput": {
"hookEventName": "PermissionRequest",
"decision": {
"behavior": "deny",
"message": "should be skipped"
}
}
}),
);
let invocation = McpInvocation {
server: "memory".to_string(),
tool: "create_entities".to_string(),
arguments: Some(serde_json::json!({ "entities": [] })),
};
let metadata = McpToolApprovalMetadata {
annotations: Some(annotations(
Some(false),
Some(true),
/*open_world*/ None,
)),
connector_id: None,
connector_name: None,
connector_description: None,
tool_title: Some("Create entities".to_string()),
tool_description: None,
mcp_app_resource_uri: None,
codex_apps_meta: None,
openai_file_input_params: None,
};
let remembered_key =
session_mcp_tool_approval_key(&invocation, Some(&metadata), AppToolApproval::Auto)
.expect("memory MCP tool should support session approval");
remember_mcp_tool_approval(&session, remembered_key).await;
let session = Arc::new(session);
let turn_context = Arc::new(turn_context);
let decision = maybe_request_mcp_tool_approval(
&session,
&turn_context,
"call-mcp-remembered",
&invocation,
"mcp__memory__create_entities",
Some(&metadata),
AppToolApproval::Auto,
)
.await;
assert_eq!(decision, Some(McpToolApprovalDecision::Accept));
assert!(
!log_path.exists(),
"remembered approval should skip PermissionRequest hooks"
);
}
#[tokio::test]
async fn guardian_mode_mcp_denial_returns_rationale_message() {
let server = start_mock_server().await;
@@ -1528,6 +1804,7 @@ async fn guardian_mode_mcp_denial_returns_rationale_message() {
&turn_context,
"call-guardian-deny",
&invocation,
"mcp__test__tool",
Some(&metadata),
AppToolApproval::Auto,
)
@@ -1584,6 +1861,7 @@ async fn prompt_mode_waits_for_approval_when_annotations_do_not_require_approval
&turn_context,
"call-prompt",
&invocation,
"mcp__test__tool",
Some(&metadata),
AppToolApproval::Prompt,
)
@@ -1658,6 +1936,7 @@ async fn approve_mode_blocks_when_arc_returns_interrupt_for_model() {
&turn_context,
"call-2",
&invocation,
"mcp__test__tool",
Some(&metadata),
AppToolApproval::Approve,
)
@@ -1729,6 +2008,7 @@ async fn custom_approve_mode_blocks_when_arc_returns_interrupt_for_model() {
&turn_context,
"call-2-custom",
&invocation,
"mcp__test__tool",
Some(&metadata),
AppToolApproval::Approve,
)
@@ -1800,6 +2080,7 @@ async fn approve_mode_blocks_when_arc_returns_interrupt_without_annotations() {
&turn_context,
"call-3",
&invocation,
"mcp__test__tool",
Some(&metadata),
AppToolApproval::Approve,
)
@@ -1884,6 +2165,7 @@ async fn full_access_mode_skips_arc_monitor_for_all_approval_modes() {
&turn_context,
"call-2",
&invocation,
"mcp__test__tool",
Some(&metadata),
approval_mode,
)
@@ -1985,6 +2267,7 @@ async fn approve_mode_routes_arc_ask_user_to_guardian_when_guardian_reviewer_is_
&turn_context,
"call-3",
&invocation,
"mcp__test__tool",
Some(&metadata),
AppToolApproval::Approve,
)
+5
View File
@@ -132,6 +132,7 @@ impl ToolOutput for CallToolResult {
#[derive(Clone, Debug)]
pub struct McpToolOutput {
pub result: CallToolResult,
pub tool_input: JsonValue,
pub wall_time: Duration,
pub original_image_detail_supported: bool,
}
@@ -162,6 +163,10 @@ impl ToolOutput for McpToolOutput {
JsonValue::String(format!("failed to serialize mcp result: {err}"))
})
}
fn post_tool_use_response(&self, _call_id: &str, _payload: &ToolPayload) -> Option<JsonValue> {
serde_json::to_value(&self.result).ok()
}
}
impl McpToolOutput {
+3
View File
@@ -98,6 +98,7 @@ fn mcp_tool_output_response_item_includes_wall_time() {
is_error: Some(false),
meta: None,
},
tool_input: json!({}),
wall_time: std::time::Duration::from_millis(1250),
original_image_detail_supported: false,
};
@@ -150,6 +151,7 @@ fn mcp_tool_output_response_item_preserves_content_items() {
is_error: Some(false),
meta: None,
},
tool_input: json!({}),
wall_time: std::time::Duration::from_millis(500),
original_image_detail_supported: false,
};
@@ -203,6 +205,7 @@ fn mcp_tool_output_code_mode_result_stays_raw_call_tool_result() {
is_error: Some(false),
meta: None,
},
tool_input: json!({}),
wall_time: std::time::Duration::from_millis(1250),
original_image_detail_supported: false,
};
@@ -316,21 +316,23 @@ impl ToolHandler for ApplyPatchHandler {
fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option<PreToolUsePayload> {
apply_patch_payload_command(&invocation.payload).map(|command| PreToolUsePayload {
tool_name: HookToolName::apply_patch(),
command,
tool_input: serde_json::json!({ "command": command }),
})
}
fn post_tool_use_payload(
&self,
call_id: &str,
payload: &ToolPayload,
invocation: &ToolInvocation,
result: &Self::Output,
) -> Option<PostToolUsePayload> {
let tool_response = result.post_tool_use_response(call_id, payload)?;
let tool_response =
result.post_tool_use_response(&invocation.call_id, &invocation.payload)?;
Some(PostToolUsePayload {
tool_name: HookToolName::apply_patch(),
tool_use_id: call_id.to_string(),
command: apply_patch_payload_command(payload)?,
tool_use_id: invocation.call_id.clone(),
tool_input: serde_json::json!({
"command": apply_patch_payload_command(&invocation.payload)?,
}),
tool_response,
})
}
@@ -54,7 +54,7 @@ async fn pre_tool_use_payload_uses_json_patch_input() {
handler.pre_tool_use_payload(&invocation),
Some(PreToolUsePayload {
tool_name: HookToolName::apply_patch(),
command: patch.to_string(),
tool_input: json!({ "command": patch }),
})
);
}
@@ -72,26 +72,27 @@ async fn pre_tool_use_payload_uses_freeform_patch_input() {
handler.pre_tool_use_payload(&invocation),
Some(PreToolUsePayload {
tool_name: HookToolName::apply_patch(),
command: patch.to_string(),
tool_input: json!({ "command": patch }),
})
);
}
#[test]
fn post_tool_use_payload_uses_patch_input_and_tool_output() {
#[tokio::test]
async fn post_tool_use_payload_uses_patch_input_and_tool_output() {
let patch = sample_patch();
let payload = ToolPayload::Custom {
input: patch.to_string(),
};
let invocation = invocation_for_payload(payload).await;
let output = ApplyPatchToolOutput::from_text("Success. Updated files.".to_string());
let handler = ApplyPatchHandler;
assert_eq!(
handler.post_tool_use_payload("call-apply-patch", &payload, &output),
handler.post_tool_use_payload(&invocation, &output),
Some(PostToolUsePayload {
tool_name: HookToolName::apply_patch(),
tool_use_id: "call-apply-patch".to_string(),
command: patch.to_string(),
tool_input: json!({ "command": patch }),
tool_response: json!("Success. Updated files."),
})
);
+156 -1
View File
@@ -6,9 +6,14 @@ use crate::mcp_tool_call::handle_mcp_tool_call;
use crate::original_image_detail::can_request_original_image_detail;
use crate::tools::context::McpToolOutput;
use crate::tools::context::ToolInvocation;
use crate::tools::context::ToolOutput;
use crate::tools::context::ToolPayload;
use crate::tools::hook_names::HookToolName;
use crate::tools::registry::PostToolUsePayload;
use crate::tools::registry::PreToolUsePayload;
use crate::tools::registry::ToolHandler;
use crate::tools::registry::ToolKind;
use serde_json::Value;
pub struct McpHandler;
impl ToolHandler for McpHandler {
@@ -18,11 +23,42 @@ impl ToolHandler for McpHandler {
ToolKind::Mcp
}
fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option<PreToolUsePayload> {
let ToolPayload::Mcp { raw_arguments, .. } = &invocation.payload else {
return None;
};
Some(PreToolUsePayload {
tool_name: HookToolName::new(invocation.tool_name.display()),
tool_input: mcp_hook_tool_input(raw_arguments),
})
}
fn post_tool_use_payload(
&self,
invocation: &ToolInvocation,
result: &Self::Output,
) -> Option<PostToolUsePayload> {
let ToolPayload::Mcp { .. } = &invocation.payload else {
return None;
};
let tool_response =
result.post_tool_use_response(&invocation.call_id, &invocation.payload)?;
Some(PostToolUsePayload {
tool_name: HookToolName::new(invocation.tool_name.display()),
tool_use_id: invocation.call_id.clone(),
tool_input: result.tool_input.clone(),
tool_response,
})
}
async fn handle(&self, invocation: ToolInvocation) -> Result<Self::Output, FunctionCallError> {
let ToolInvocation {
session,
turn,
call_id,
tool_name: model_tool_name,
payload,
..
} = invocation;
@@ -50,14 +86,133 @@ impl ToolHandler for McpHandler {
call_id.clone(),
server,
tool,
model_tool_name.display(),
arguments_str,
)
.await;
Ok(McpToolOutput {
result,
result: result.result,
tool_input: result.tool_input,
wall_time: started.elapsed(),
original_image_detail_supported: can_request_original_image_detail(&turn.model_info),
})
}
}
fn mcp_hook_tool_input(raw_arguments: &str) -> Value {
if raw_arguments.trim().is_empty() {
return Value::Object(serde_json::Map::new());
}
serde_json::from_str(raw_arguments).unwrap_or_else(|_| Value::String(raw_arguments.to_string()))
}
#[cfg(test)]
mod tests {
use super::*;
use crate::session::tests::make_session_and_context;
use crate::turn_diff_tracker::TurnDiffTracker;
use pretty_assertions::assert_eq;
use serde_json::json;
use std::time::Duration;
use tokio::sync::Mutex;
#[tokio::test]
async fn mcp_pre_tool_use_payload_uses_model_tool_name_and_raw_args() {
let payload = ToolPayload::Mcp {
server: "memory".to_string(),
tool: "create_entities".to_string(),
raw_arguments: json!({
"entities": [{
"name": "Ada",
"entityType": "person"
}]
})
.to_string(),
};
let (session, turn) = make_session_and_context().await;
assert_eq!(
McpHandler.pre_tool_use_payload(&ToolInvocation {
session: session.into(),
turn: turn.into(),
cancellation_token: tokio_util::sync::CancellationToken::new(),
tracker: Arc::new(Mutex::new(TurnDiffTracker::new())),
call_id: "call-mcp-pre".to_string(),
tool_name: codex_tools::ToolName::namespaced("mcp__memory__", "create_entities"),
payload,
}),
Some(PreToolUsePayload {
tool_name: HookToolName::new("mcp__memory__create_entities"),
tool_input: json!({
"entities": [{
"name": "Ada",
"entityType": "person"
}]
}),
})
);
}
#[tokio::test]
async fn mcp_post_tool_use_payload_uses_model_tool_name_args_and_result() {
let payload = ToolPayload::Mcp {
server: "filesystem".to_string(),
tool: "read_file".to_string(),
raw_arguments: json!({ "path": "/tmp/notes.txt" }).to_string(),
};
let output = McpToolOutput {
result: codex_protocol::mcp::CallToolResult {
content: vec![json!({
"type": "text",
"text": "notes"
})],
structured_content: Some(json!({ "bytes": 5 })),
is_error: None,
meta: None,
},
tool_input: json!({
"path": {
"file_id": "file_123"
}
}),
wall_time: Duration::from_millis(42),
original_image_detail_supported: true,
};
let (session, turn) = make_session_and_context().await;
let invocation = ToolInvocation {
session: session.into(),
turn: turn.into(),
cancellation_token: tokio_util::sync::CancellationToken::new(),
tracker: Arc::new(Mutex::new(TurnDiffTracker::new())),
call_id: "call-mcp-post".to_string(),
tool_name: codex_tools::ToolName::namespaced("mcp__filesystem__", "read_file"),
payload,
};
assert_eq!(
McpHandler.post_tool_use_payload(&invocation, &output),
Some(PostToolUsePayload {
tool_name: HookToolName::new("mcp__filesystem__read_file"),
tool_use_id: "call-mcp-post".to_string(),
tool_input: json!({
"path": {
"file_id": "file_123"
}
}),
tool_response: json!({
"content": [{
"type": "text",
"text": "notes"
}],
"structuredContent": { "bytes": 5 }
}),
})
);
}
#[test]
fn mcp_hook_tool_input_defaults_empty_args_to_object() {
assert_eq!(mcp_hook_tool_input(" "), json!({}));
}
}
@@ -5,8 +5,6 @@ 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 crate::context::ContextualUserFragment;
use crate::context::SpawnAgentInstructions;
use codex_protocol::AgentPath;
use codex_protocol::protocol::InterAgentCommunication;
use codex_protocol::protocol::Op;
@@ -88,18 +86,6 @@ impl ToolHandler for Handler {
}
apply_spawn_agent_runtime_overrides(&mut config, turn.as_ref())?;
apply_spawn_agent_overrides(&mut config, child_depth);
let spawn_agent_instructions = SpawnAgentInstructions.render();
config.developer_instructions = Some(
if let Some(mut existing_instructions) = config.developer_instructions.take() {
if !existing_instructions.ends_with('\n') {
existing_instructions.push('\n');
}
existing_instructions.push_str(&spawn_agent_instructions);
existing_instructions
} else {
spawn_agent_instructions
},
);
let spawn_source = thread_spawn_source(
session.conversation_id,
+14 -12
View File
@@ -208,21 +208,22 @@ impl ToolHandler for ShellHandler {
fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option<PreToolUsePayload> {
shell_payload_command(&invocation.payload).map(|command| PreToolUsePayload {
tool_name: HookToolName::bash(),
command,
tool_input: serde_json::json!({ "command": command }),
})
}
fn post_tool_use_payload(
&self,
call_id: &str,
payload: &ToolPayload,
invocation: &ToolInvocation,
result: &Self::Output,
) -> Option<PostToolUsePayload> {
let tool_response = result.post_tool_use_response(call_id, payload)?;
let tool_response =
result.post_tool_use_response(&invocation.call_id, &invocation.payload)?;
let command = shell_payload_command(&invocation.payload)?;
Some(PostToolUsePayload {
tool_name: HookToolName::bash(),
tool_use_id: call_id.to_string(),
command: shell_payload_command(payload)?,
tool_use_id: invocation.call_id.clone(),
tool_input: serde_json::json!({ "command": command }),
tool_response,
})
}
@@ -321,21 +322,22 @@ impl ToolHandler for ShellCommandHandler {
fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option<PreToolUsePayload> {
shell_command_payload_command(&invocation.payload).map(|command| PreToolUsePayload {
tool_name: HookToolName::bash(),
command,
tool_input: serde_json::json!({ "command": command }),
})
}
fn post_tool_use_payload(
&self,
call_id: &str,
payload: &ToolPayload,
invocation: &ToolInvocation,
result: &Self::Output,
) -> Option<PostToolUsePayload> {
let tool_response = result.post_tool_use_response(call_id, payload)?;
let tool_response =
result.post_tool_use_response(&invocation.call_id, &invocation.payload)?;
let command = shell_command_payload_command(&invocation.payload)?;
Some(PostToolUsePayload {
tool_name: HookToolName::bash(),
tool_use_id: call_id.to_string(),
command: shell_command_payload_command(payload)?,
tool_use_id: invocation.call_id.clone(),
tool_input: serde_json::json!({ "command": command }),
tool_response,
})
}
@@ -234,7 +234,7 @@ async fn shell_pre_tool_use_payload_uses_joined_command() {
}),
Some(crate::tools::registry::PreToolUsePayload {
tool_name: HookToolName::bash(),
command: "bash -lc 'printf hi'".to_string(),
tool_input: json!({ "command": "bash -lc 'printf hi'" }),
})
);
}
@@ -261,13 +261,13 @@ async fn shell_command_pre_tool_use_payload_uses_raw_command() {
}),
Some(crate::tools::registry::PreToolUsePayload {
tool_name: HookToolName::bash(),
command: "printf shell command".to_string(),
tool_input: json!({ "command": "printf shell command" }),
})
);
}
#[test]
fn build_post_tool_use_payload_uses_tool_output_wire_value() {
#[tokio::test]
async fn build_post_tool_use_payload_uses_tool_output_wire_value() {
let payload = ToolPayload::Function {
arguments: json!({ "command": "printf shell command" }).to_string(),
};
@@ -279,13 +279,22 @@ fn build_post_tool_use_payload_uses_tool_output_wire_value() {
let handler = ShellCommandHandler {
backend: super::ShellCommandBackend::Classic,
};
let (session, turn) = make_session_and_context().await;
let invocation = ToolInvocation {
session: session.into(),
turn: turn.into(),
cancellation_token: tokio_util::sync::CancellationToken::new(),
tracker: Arc::new(Mutex::new(TurnDiffTracker::new())),
call_id: "call-42".to_string(),
tool_name: codex_tools::ToolName::plain("shell_command"),
payload,
};
assert_eq!(
handler.post_tool_use_payload("call-42", &payload, &output),
handler.post_tool_use_payload(&invocation, &output),
Some(crate::tools::registry::PostToolUsePayload {
tool_name: HookToolName::bash(),
tool_use_id: "call-42".to_string(),
command: "printf shell command".to_string(),
tool_input: json!({ "command": "printf shell command" }),
tool_response: json!("shell output"),
})
);
@@ -139,31 +139,30 @@ impl ToolHandler for UnifiedExecHandler {
.ok()
.map(|args| PreToolUsePayload {
tool_name: HookToolName::bash(),
command: args.cmd,
tool_input: serde_json::json!({ "command": args.cmd }),
})
}
fn post_tool_use_payload(
&self,
call_id: &str,
payload: &ToolPayload,
invocation: &ToolInvocation,
result: &Self::Output,
) -> Option<PostToolUsePayload> {
let ToolPayload::Function { .. } = payload else {
let ToolPayload::Function { .. } = &invocation.payload else {
return None;
};
let command = result.hook_command.clone()?;
let tool_use_id = if result.event_call_id.is_empty() {
call_id.to_string()
invocation.call_id.clone()
} else {
result.event_call_id.clone()
};
let tool_response = result.post_tool_use_response(&tool_use_id, payload)?;
let tool_response = result.post_tool_use_response(&tool_use_id, &invocation.payload)?;
Some(PostToolUsePayload {
tool_name: HookToolName::bash(),
tool_use_id,
command,
tool_input: serde_json::json!({ "command": command }),
tool_response,
})
}
@@ -22,6 +22,23 @@ use crate::tools::registry::ToolHandler;
use crate::turn_diff_tracker::TurnDiffTracker;
use tokio::sync::Mutex;
async fn invocation_for_payload(
tool_name: &str,
call_id: &str,
payload: ToolPayload,
) -> ToolInvocation {
let (session, turn) = make_session_and_context().await;
ToolInvocation {
session: session.into(),
turn: turn.into(),
cancellation_token: tokio_util::sync::CancellationToken::new(),
tracker: Arc::new(Mutex::new(TurnDiffTracker::new())),
call_id: call_id.to_string(),
tool_name: codex_tools::ToolName::plain(tool_name),
payload,
}
}
#[test]
fn test_get_command_uses_default_shell_when_unspecified() -> anyhow::Result<()> {
let json = r#"{"cmd": "echo hello"}"#;
@@ -219,7 +236,7 @@ async fn exec_command_pre_tool_use_payload_uses_raw_command() {
}),
Some(crate::tools::registry::PreToolUsePayload {
tool_name: HookToolName::bash(),
command: "printf exec command".to_string(),
tool_input: serde_json::json!({ "command": "printf exec command" }),
})
);
}
@@ -246,8 +263,8 @@ async fn exec_command_pre_tool_use_payload_skips_write_stdin() {
);
}
#[test]
fn exec_command_post_tool_use_payload_uses_output_for_noninteractive_one_shot_commands() {
#[tokio::test]
async fn exec_command_post_tool_use_payload_uses_output_for_noninteractive_one_shot_commands() {
let payload = ToolPayload::Function {
arguments: serde_json::json!({ "cmd": "echo three", "tty": false }).to_string(),
};
@@ -262,20 +279,20 @@ fn exec_command_post_tool_use_payload_uses_output_for_noninteractive_one_shot_co
original_token_count: None,
hook_command: Some("echo three".to_string()),
};
let invocation = invocation_for_payload("exec_command", "call-43", payload).await;
assert_eq!(
UnifiedExecHandler.post_tool_use_payload("call-43", &payload, &output),
UnifiedExecHandler.post_tool_use_payload(&invocation, &output),
Some(crate::tools::registry::PostToolUsePayload {
tool_name: HookToolName::bash(),
tool_use_id: "call-43".to_string(),
command: "echo three".to_string(),
tool_input: serde_json::json!({ "command": "echo three" }),
tool_response: serde_json::json!("three"),
})
);
}
#[test]
fn exec_command_post_tool_use_payload_uses_output_for_interactive_completion() {
#[tokio::test]
async fn exec_command_post_tool_use_payload_uses_output_for_interactive_completion() {
let payload = ToolPayload::Function {
arguments: serde_json::json!({ "cmd": "echo three", "tty": true }).to_string(),
};
@@ -290,20 +307,21 @@ fn exec_command_post_tool_use_payload_uses_output_for_interactive_completion() {
original_token_count: None,
hook_command: Some("echo three".to_string()),
};
let invocation = invocation_for_payload("exec_command", "call-44", payload).await;
assert_eq!(
UnifiedExecHandler.post_tool_use_payload("call-44", &payload, &output),
UnifiedExecHandler.post_tool_use_payload(&invocation, &output),
Some(crate::tools::registry::PostToolUsePayload {
tool_name: HookToolName::bash(),
tool_use_id: "call-44".to_string(),
command: "echo three".to_string(),
tool_input: serde_json::json!({ "command": "echo three" }),
tool_response: serde_json::json!("three"),
})
);
}
#[test]
fn exec_command_post_tool_use_payload_skips_running_sessions() {
#[tokio::test]
async fn exec_command_post_tool_use_payload_skips_running_sessions() {
let payload = ToolPayload::Function {
arguments: serde_json::json!({ "cmd": "echo three", "tty": false }).to_string(),
};
@@ -318,15 +336,15 @@ fn exec_command_post_tool_use_payload_skips_running_sessions() {
original_token_count: None,
hook_command: Some("echo three".to_string()),
};
let invocation = invocation_for_payload("exec_command", "call-45", payload).await;
assert_eq!(
UnifiedExecHandler.post_tool_use_payload("call-45", &payload, &output),
UnifiedExecHandler.post_tool_use_payload(&invocation, &output),
None
);
}
#[test]
fn write_stdin_post_tool_use_payload_uses_original_exec_call_id_and_command_on_completion() {
#[tokio::test]
async fn write_stdin_post_tool_use_payload_uses_original_exec_call_id_and_command_on_completion() {
let payload = ToolPayload::Function {
arguments: serde_json::json!({
"session_id": 45,
@@ -345,20 +363,21 @@ fn write_stdin_post_tool_use_payload_uses_original_exec_call_id_and_command_on_c
original_token_count: None,
hook_command: Some("sleep 1; echo finished".to_string()),
};
let invocation = invocation_for_payload("write_stdin", "write-stdin-call", payload).await;
assert_eq!(
UnifiedExecHandler.post_tool_use_payload("write-stdin-call", &payload, &output),
UnifiedExecHandler.post_tool_use_payload(&invocation, &output),
Some(crate::tools::registry::PostToolUsePayload {
tool_name: HookToolName::bash(),
tool_use_id: "exec-call-45".to_string(),
command: "sleep 1; echo finished".to_string(),
tool_input: serde_json::json!({ "command": "sleep 1; echo finished" }),
tool_response: serde_json::json!("finished\n"),
})
);
}
#[test]
fn write_stdin_post_tool_use_payload_keeps_parallel_session_metadata_separate() {
#[tokio::test]
async fn write_stdin_post_tool_use_payload_keeps_parallel_session_metadata_separate() {
let payload = ToolPayload::Function {
arguments: serde_json::json!({ "session_id": 45, "chars": "" }).to_string(),
};
@@ -384,10 +403,12 @@ fn write_stdin_post_tool_use_payload_keeps_parallel_session_metadata_separate()
original_token_count: None,
hook_command: Some("sleep 1; echo beta".to_string()),
};
let invocation_b = invocation_for_payload("write_stdin", "write-call-b", payload.clone()).await;
let invocation_a = invocation_for_payload("write_stdin", "write-call-a", payload).await;
let payloads = [
UnifiedExecHandler.post_tool_use_payload("write-call-b", &payload, &output_b),
UnifiedExecHandler.post_tool_use_payload("write-call-a", &payload, &output_a),
UnifiedExecHandler.post_tool_use_payload(&invocation_b, &output_b),
UnifiedExecHandler.post_tool_use_payload(&invocation_a, &output_a),
];
assert_eq!(
@@ -396,13 +417,13 @@ fn write_stdin_post_tool_use_payload_keeps_parallel_session_metadata_separate()
Some(crate::tools::registry::PostToolUsePayload {
tool_name: HookToolName::bash(),
tool_use_id: "exec-call-b".to_string(),
command: "sleep 1; echo beta".to_string(),
tool_input: serde_json::json!({ "command": "sleep 1; echo beta" }),
tool_response: serde_json::json!("beta\n"),
}),
Some(crate::tools::registry::PostToolUsePayload {
tool_name: HookToolName::bash(),
tool_use_id: "exec-call-a".to_string(),
command: "sleep 2; echo alpha".to_string(),
tool_input: serde_json::json!({ "command": "sleep 2; echo alpha" }),
tool_response: serde_json::json!("alpha\n"),
}),
]
+1 -6
View File
@@ -8,7 +8,6 @@ use crate::guardian::routes_approval_to_guardian;
use crate::hook_runtime::run_permission_request_hooks;
use crate::network_policy_decision::denied_network_policy_message;
use crate::session::session::Session;
use crate::tools::hook_names::HookToolName;
use crate::tools::sandboxing::PermissionRequestPayload;
use crate::tools::sandboxing::ToolError;
use codex_hooks::PermissionRequestDecision;
@@ -393,11 +392,7 @@ impl NetworkApprovalService {
&session,
&turn_context,
&guardian_approval_id,
PermissionRequestPayload {
tool_name: HookToolName::bash(),
command,
description: Some(format!("network-access {target}")),
},
PermissionRequestPayload::bash(command, Some(format!("network-access {target}"))),
)
.await
{
+15 -17
View File
@@ -70,8 +70,7 @@ pub trait ToolHandler: Send + Sync {
fn post_tool_use_payload(
&self,
_call_id: &str,
_payload: &ToolPayload,
_invocation: &ToolInvocation,
_result: &Self::Output,
) -> Option<PostToolUsePayload> {
None
@@ -136,8 +135,11 @@ pub(crate) struct PreToolUsePayload {
/// The canonical name is serialized to hook stdin, while aliases are used
/// only for matcher compatibility.
pub(crate) tool_name: HookToolName,
/// Command-shaped input exposed at `tool_input.command`.
pub(crate) command: String,
/// Tool-specific input exposed at `tool_input`.
///
/// Shell-like tools use `{ "command": ... }`; MCP tools use their resolved
/// JSON arguments.
pub(crate) tool_input: Value,
}
#[derive(Debug, Clone, PartialEq)]
@@ -149,8 +151,8 @@ pub(crate) struct PostToolUsePayload {
pub(crate) tool_name: HookToolName,
/// The originating tool-use id exposed at `tool_use_id`.
pub(crate) tool_use_id: String,
/// Command-shaped input exposed at `tool_input.command`.
pub(crate) command: String,
/// Tool-specific input exposed at `tool_input`.
pub(crate) tool_input: Value,
/// Tool result exposed at `tool_response`.
pub(crate) tool_response: Value,
}
@@ -195,9 +197,9 @@ where
Box::pin(async move {
let call_id = invocation.call_id.clone();
let payload = invocation.payload.clone();
let output = self.handle(invocation).await?;
let output = self.handle(invocation.clone()).await?;
let post_tool_use_payload =
ToolHandler::post_tool_use_payload(self, &call_id, &payload, &output);
ToolHandler::post_tool_use_payload(self, &invocation, &output);
Ok(AnyToolResult {
call_id,
payload,
@@ -328,20 +330,16 @@ impl ToolRegistry {
}
if let Some(pre_tool_use_payload) = handler.pre_tool_use_payload(&invocation)
&& let Some(reason) = run_pre_tool_use_hooks(
&& let Some(message) = run_pre_tool_use_hooks(
&invocation.session,
&invocation.turn,
invocation.call_id.clone(),
pre_tool_use_payload.tool_name.name().to_string(),
pre_tool_use_payload.tool_name.matcher_aliases().to_vec(),
pre_tool_use_payload.command.clone(),
&pre_tool_use_payload.tool_name,
&pre_tool_use_payload.tool_input,
)
.await
{
return Err(FunctionCallError::RespondToModel(format!(
"Command blocked by PreToolUse hook: {reason}. Command: {}",
pre_tool_use_payload.command
)));
return Err(FunctionCallError::RespondToModel(message));
}
let is_mutating = handler.is_mutating(&invocation).await;
@@ -402,7 +400,7 @@ impl ToolRegistry {
post_tool_use_payload.tool_use_id,
post_tool_use_payload.tool_name.name().to_string(),
post_tool_use_payload.tool_name.matcher_aliases().to_vec(),
post_tool_use_payload.command,
post_tool_use_payload.tool_input,
post_tool_use_payload.tool_response,
)
.await,
@@ -204,8 +204,7 @@ impl Approvable<ApplyPatchRequest> for ApplyPatchRuntime {
) -> Option<PermissionRequestPayload> {
Some(PermissionRequestPayload {
tool_name: HookToolName::apply_patch(),
command: req.action.patch.clone(),
description: None,
tool_input: serde_json::json!({ "command": req.action.patch }),
})
}
}
@@ -105,8 +105,10 @@ fn permission_request_payload_uses_apply_patch_hook_name_and_aliases() {
payload.tool_name.matcher_aliases(),
&["Write".to_string(), "Edit".to_string()]
);
assert_eq!(payload.command, expected_patch);
assert_eq!(payload.description, None);
assert_eq!(
payload.tool_input,
serde_json::json!({ "command": expected_patch })
);
}
#[test]
+4 -6
View File
@@ -17,7 +17,6 @@ use crate::sandboxing::ExecOptions;
use crate::sandboxing::SandboxPermissions;
use crate::sandboxing::execute_env;
use crate::shell::ShellType;
use crate::tools::hook_names::HookToolName;
use crate::tools::network_approval::NetworkApprovalMode;
use crate::tools::network_approval::NetworkApprovalSpec;
use crate::tools::runtimes::build_sandbox_command;
@@ -202,11 +201,10 @@ impl Approvable<ShellRequest> for ShellRuntime {
}
fn permission_request_payload(&self, req: &ShellRequest) -> Option<PermissionRequestPayload> {
Some(PermissionRequestPayload {
tool_name: HookToolName::bash(),
command: req.hook_command.clone(),
description: req.justification.clone(),
})
Some(PermissionRequestPayload::bash(
req.hook_command.clone(),
req.justification.clone(),
))
}
fn sandbox_mode_for_first_attempt(&self, req: &ShellRequest) -> SandboxOverride {
@@ -13,7 +13,6 @@ use crate::sandboxing::ExecOptions;
use crate::sandboxing::ExecRequest;
use crate::sandboxing::SandboxPermissions;
use crate::shell::ShellType;
use crate::tools::hook_names::HookToolName;
use crate::tools::runtimes::build_sandbox_command;
use crate::tools::sandboxing::PermissionRequestPayload;
use crate::tools::sandboxing::SandboxAttempt;
@@ -406,11 +405,10 @@ impl CoreShellActionProvider {
Ok(stopwatch
.pause_for(async move {
// 1) Run PermissionRequest hooks
let permission_request = PermissionRequestPayload {
tool_name: HookToolName::bash(),
command: codex_shell_command::parse_command::shlex_join(&command),
description: None,
};
let permission_request = PermissionRequestPayload::bash(
codex_shell_command::parse_command::shlex_join(&command),
/*description*/ None,
);
let effective_approval_id = approval_id.clone().unwrap_or_else(|| call_id.clone());
match run_permission_request_hooks(
&session,
@@ -14,7 +14,6 @@ use crate::sandboxing::ExecOptions;
use crate::sandboxing::ExecServerEnvConfig;
use crate::sandboxing::SandboxPermissions;
use crate::shell::ShellType;
use crate::tools::hook_names::HookToolName;
use crate::tools::network_approval::NetworkApprovalMode;
use crate::tools::network_approval::NetworkApprovalSpec;
use crate::tools::runtimes::build_sandbox_command;
@@ -187,11 +186,10 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
&self,
req: &UnifiedExecRequest,
) -> Option<PermissionRequestPayload> {
Some(PermissionRequestPayload {
tool_name: HookToolName::bash(),
command: req.hook_command.clone(),
description: req.justification.clone(),
})
Some(PermissionRequestPayload::bash(
req.hook_command.clone(),
req.justification.clone(),
))
}
fn sandbox_mode_for_first_attempt(&self, req: &UnifiedExecRequest) -> SandboxOverride {
+19 -2
View File
@@ -135,8 +135,25 @@ pub(crate) struct ApprovalCtx<'a> {
#[derive(Clone, Debug, PartialEq, Eq)]
pub(crate) struct PermissionRequestPayload {
pub tool_name: HookToolName,
pub command: String,
pub description: Option<String>,
pub tool_input: serde_json::Value,
}
impl PermissionRequestPayload {
pub(crate) fn bash(command: String, description: Option<String>) -> Self {
let mut tool_input = serde_json::Map::new();
tool_input.insert("command".to_string(), serde_json::Value::String(command));
if let Some(description) = description {
tool_input.insert(
"description".to_string(),
serde_json::Value::String(description),
);
}
Self {
tool_name: HookToolName::bash(),
tool_input: serde_json::Value::Object(tool_input),
}
}
}
// Specifies what tool orchestrator should do with a given tool call.
@@ -1,8 +1,38 @@
use super::*;
use crate::sandboxing::SandboxPermissions;
use crate::tools::hook_names::HookToolName;
use codex_protocol::protocol::GranularApprovalConfig;
use codex_protocol::protocol::NetworkAccess;
use pretty_assertions::assert_eq;
use serde_json::json;
#[test]
fn bash_permission_request_payload_omits_missing_description() {
assert_eq!(
PermissionRequestPayload::bash("echo hi".to_string(), /*description*/ None),
PermissionRequestPayload {
tool_name: HookToolName::bash(),
tool_input: json!({ "command": "echo hi" }),
}
);
}
#[test]
fn bash_permission_request_payload_includes_description_when_present() {
assert_eq!(
PermissionRequestPayload::bash(
"echo hi".to_string(),
Some("network-access example.com".to_string()),
),
PermissionRequestPayload {
tool_name: HookToolName::bash(),
tool_input: json!({
"command": "echo hi",
"description": "network-access example.com",
}),
}
);
}
#[test]
fn external_sandbox_skips_exec_approval_on_request() {
+350
View File
@@ -0,0 +1,350 @@
use std::collections::HashMap;
use std::fs;
use std::path::Path;
use std::time::Duration;
use anyhow::Context;
use anyhow::Result;
use codex_config::types::AppToolApproval;
use codex_config::types::McpServerConfig;
use codex_config::types::McpServerTransportConfig;
use codex_core::config::Config;
use codex_features::Feature;
use core_test_support::responses::ev_assistant_message;
use core_test_support::responses::ev_completed;
use core_test_support::responses::ev_function_call_with_namespace;
use core_test_support::responses::ev_response_created;
use core_test_support::responses::mount_sse_once;
use core_test_support::responses::mount_sse_sequence;
use core_test_support::responses::sse;
use core_test_support::responses::start_mock_server;
use core_test_support::skip_if_no_network;
use core_test_support::stdio_server_bin;
use core_test_support::test_codex::test_codex;
use pretty_assertions::assert_eq;
use serde_json::Value;
use serde_json::json;
const RMCP_SERVER: &str = "rmcp";
const RMCP_NAMESPACE: &str = "mcp__rmcp__";
const RMCP_ECHO_TOOL_NAME: &str = "mcp__rmcp__echo";
const RMCP_HOOK_MATCHER: &str = "mcp__rmcp__.*";
const RMCP_ECHO_MESSAGE: &str = "hook e2e ping";
fn write_pre_tool_use_hook(home: &Path, reason: &str) -> Result<()> {
let script_path = home.join("pre_tool_use_hook.py");
let log_path = home.join("pre_tool_use_hook_log.jsonl");
let reason_json = serde_json::to_string(reason).context("serialize pre tool use reason")?;
let script = format!(
r#"import json
from pathlib import Path
import sys
payload = json.load(sys.stdin)
with Path(r"{log_path}").open("a", encoding="utf-8") as handle:
handle.write(json.dumps(payload) + "\n")
print(json.dumps({{
"hookSpecificOutput": {{
"hookEventName": "PreToolUse",
"permissionDecision": "deny",
"permissionDecisionReason": {reason_json}
}}
}}))
"#,
log_path = log_path.display(),
reason_json = reason_json,
);
let hooks = serde_json::json!({
"hooks": {
"PreToolUse": [{
"matcher": RMCP_HOOK_MATCHER,
"hooks": [{
"type": "command",
"command": format!("python3 {}", script_path.display()),
"statusMessage": "running MCP pre tool use hook",
}]
}]
}
});
fs::write(&script_path, script).context("write pre tool use hook script")?;
fs::write(home.join("hooks.json"), hooks.to_string()).context("write hooks.json")?;
Ok(())
}
fn write_post_tool_use_hook(home: &Path, additional_context: &str) -> Result<()> {
let script_path = home.join("post_tool_use_hook.py");
let log_path = home.join("post_tool_use_hook_log.jsonl");
let additional_context_json =
serde_json::to_string(additional_context).context("serialize post tool use context")?;
let script = format!(
r#"import json
from pathlib import Path
import sys
payload = json.load(sys.stdin)
with Path(r"{log_path}").open("a", encoding="utf-8") as handle:
handle.write(json.dumps(payload) + "\n")
print(json.dumps({{
"hookSpecificOutput": {{
"hookEventName": "PostToolUse",
"additionalContext": {additional_context_json}
}}
}}))
"#,
log_path = log_path.display(),
additional_context_json = additional_context_json,
);
let hooks = serde_json::json!({
"hooks": {
"PostToolUse": [{
"matcher": RMCP_HOOK_MATCHER,
"hooks": [{
"type": "command",
"command": format!("python3 {}", script_path.display()),
"statusMessage": "running MCP post tool use hook",
}]
}]
}
});
fs::write(&script_path, script).context("write post tool use hook script")?;
fs::write(home.join("hooks.json"), hooks.to_string()).context("write hooks.json")?;
Ok(())
}
fn read_hook_inputs(home: &Path, log_name: &str) -> Result<Vec<Value>> {
fs::read_to_string(home.join(log_name))
.with_context(|| format!("read {log_name}"))?
.lines()
.filter(|line| !line.trim().is_empty())
.map(|line| serde_json::from_str(line).with_context(|| format!("parse {log_name} line")))
.collect()
}
fn insert_rmcp_test_server(config: &mut Config, command: String, approval_mode: AppToolApproval) {
let mut servers = config.mcp_servers.get().clone();
servers.insert(
RMCP_SERVER.to_string(),
McpServerConfig {
transport: McpServerTransportConfig::Stdio {
command,
args: Vec::new(),
env: None,
env_vars: Vec::new(),
cwd: None,
},
experimental_environment: None,
enabled: true,
required: false,
supports_parallel_tool_calls: false,
disabled_reason: None,
startup_timeout_sec: Some(Duration::from_secs(10)),
tool_timeout_sec: None,
default_tools_approval_mode: Some(approval_mode),
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth_resource: None,
tools: HashMap::new(),
},
);
if let Err(err) = config.mcp_servers.set(servers) {
panic!("test mcp servers should accept any configuration: {err}");
}
}
fn enable_hooks_and_rmcp_server(
config: &mut Config,
rmcp_test_server_bin: String,
approval_mode: AppToolApproval,
) {
if let Err(err) = config.features.enable(Feature::CodexHooks) {
panic!("test config should allow feature update: {err}");
}
insert_rmcp_test_server(config, rmcp_test_server_bin, approval_mode);
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn pre_tool_use_blocks_mcp_tool_before_execution() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let call_id = "pretooluse-rmcp-echo";
let arguments = json!({ "message": RMCP_ECHO_MESSAGE }).to_string();
let responses = mount_sse_sequence(
&server,
vec![
sse(vec![
ev_response_created("resp-1"),
ev_function_call_with_namespace(call_id, RMCP_NAMESPACE, "echo", &arguments),
ev_completed("resp-1"),
]),
sse(vec![
ev_response_created("resp-2"),
ev_assistant_message("msg-1", "mcp hook blocked it"),
ev_completed("resp-2"),
]),
],
)
.await;
let block_reason = "blocked mcp pre hook";
let rmcp_test_server_bin = stdio_server_bin()?;
let test = test_codex()
.with_pre_build_hook(move |home| {
if let Err(error) = write_pre_tool_use_hook(home, block_reason) {
panic!("failed to write MCP pre tool use hook fixture: {error}");
}
})
.with_config(move |config| {
enable_hooks_and_rmcp_server(config, rmcp_test_server_bin, AppToolApproval::Approve);
})
.build(&server)
.await?;
test.submit_turn("call the rmcp echo tool with the MCP pre hook")
.await?;
let requests = responses.requests();
assert_eq!(requests.len(), 2);
let output_item = requests[1].function_call_output(call_id);
let output = output_item
.get("output")
.and_then(Value::as_str)
.expect("blocked MCP tool output string");
assert!(
output.contains(&format!(
"Tool call blocked by PreToolUse hook: {block_reason}. Tool: {RMCP_ECHO_TOOL_NAME}"
)),
"blocked MCP tool output should surface the hook reason and tool name",
);
let hook_inputs = read_hook_inputs(test.codex_home_path(), "pre_tool_use_hook_log.jsonl")?;
assert_eq!(hook_inputs.len(), 1);
assert_eq!(
json!({
"hook_event_name": hook_inputs[0]["hook_event_name"],
"tool_name": hook_inputs[0]["tool_name"],
"tool_use_id": hook_inputs[0]["tool_use_id"],
"tool_input": hook_inputs[0]["tool_input"],
}),
json!({
"hook_event_name": "PreToolUse",
"tool_name": RMCP_ECHO_TOOL_NAME,
"tool_use_id": call_id,
"tool_input": { "message": RMCP_ECHO_MESSAGE },
})
);
let transcript_path = hook_inputs[0]["transcript_path"]
.as_str()
.expect("pre tool use hook transcript_path");
assert!(
Path::new(transcript_path).exists(),
"pre tool use hook transcript_path should be materialized on disk",
);
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn post_tool_use_records_mcp_tool_payload_and_context() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let call_id = "posttooluse-rmcp-echo";
let arguments = json!({ "message": RMCP_ECHO_MESSAGE }).to_string();
let call_mock = mount_sse_once(
&server,
sse(vec![
ev_response_created("resp-1"),
ev_function_call_with_namespace(call_id, RMCP_NAMESPACE, "echo", &arguments),
ev_completed("resp-1"),
]),
)
.await;
let final_mock = mount_sse_once(
&server,
sse(vec![
ev_response_created("resp-2"),
ev_assistant_message("msg-1", "mcp post hook context observed"),
ev_completed("resp-2"),
]),
)
.await;
let post_context = "Remember the MCP post-tool note.";
let rmcp_test_server_bin = stdio_server_bin()?;
let test = test_codex()
.with_pre_build_hook(move |home| {
if let Err(error) = write_post_tool_use_hook(home, post_context) {
panic!("failed to write MCP post tool use hook fixture: {error}");
}
})
.with_config(move |config| {
enable_hooks_and_rmcp_server(config, rmcp_test_server_bin, AppToolApproval::Approve);
})
.build(&server)
.await?;
test.submit_turn("call the rmcp echo tool with the MCP post hook")
.await?;
let final_request = final_mock.single_request();
assert!(
final_request
.message_input_texts("developer")
.contains(&post_context.to_string()),
"follow-up request should include MCP post tool use additional context",
);
let output_item = final_request.function_call_output(call_id);
let output = output_item
.get("output")
.and_then(Value::as_str)
.expect("MCP tool output string");
assert!(
output.contains(&format!("ECHOING: {RMCP_ECHO_MESSAGE}")),
"MCP tool output should still reach the model",
);
let hook_inputs = read_hook_inputs(test.codex_home_path(), "post_tool_use_hook_log.jsonl")?;
assert_eq!(hook_inputs.len(), 1);
assert_eq!(
json!({
"hook_event_name": hook_inputs[0]["hook_event_name"],
"tool_name": hook_inputs[0]["tool_name"],
"tool_use_id": hook_inputs[0]["tool_use_id"],
"tool_input": hook_inputs[0]["tool_input"],
"tool_response": hook_inputs[0]["tool_response"],
}),
json!({
"hook_event_name": "PostToolUse",
"tool_name": RMCP_ECHO_TOOL_NAME,
"tool_use_id": call_id,
"tool_input": { "message": RMCP_ECHO_MESSAGE },
"tool_response": {
"content": [],
"structuredContent": {
"echo": format!("ECHOING: {RMCP_ECHO_MESSAGE}"),
"env": null,
},
"isError": false,
},
})
);
let transcript_path = hook_inputs[0]["transcript_path"]
.as_str()
.expect("post tool use hook transcript_path");
assert!(
Path::new(transcript_path).exists(),
"post tool use hook transcript_path should be materialized on disk",
);
call_mock.single_request();
Ok(())
}
+2
View File
@@ -47,6 +47,8 @@ mod fork_thread;
mod hierarchical_agents;
#[cfg(not(target_os = "windows"))]
mod hooks;
#[cfg(not(target_os = "windows"))]
mod hooks_mcp;
mod image_rollout;
mod items;
mod js_repl;
+79 -1
View File
@@ -1,5 +1,9 @@
#![cfg(not(target_os = "windows"))]
use std::fs;
use std::path::Path;
use anyhow::Context;
use anyhow::Result;
use codex_core::config::Config;
use codex_features::Feature;
@@ -28,6 +32,7 @@ use wiremock::matchers::path;
const DOCUMENT_EXTRACT_NAMESPACE: &str = "mcp__codex_apps__calendar";
const DOCUMENT_EXTRACT_TOOL: &str = "_extract_text";
const DOCUMENT_EXTRACT_HOOK_MATCHER: &str = "mcp__codex_apps__calendar_extract_text";
fn configure_apps(config: &mut Config, chatgpt_base_url: &str) {
if let Err(err) = config.features.enable(Feature::Apps) {
@@ -36,6 +41,55 @@ fn configure_apps(config: &mut Config, chatgpt_base_url: &str) {
config.chatgpt_base_url = chatgpt_base_url.to_string();
}
fn write_post_tool_use_hook(home: &Path) -> Result<()> {
let script_path = home.join("post_tool_use_hook.py");
let log_path = home.join("post_tool_use_hook_log.jsonl");
let script = format!(
r#"import json
from pathlib import Path
import sys
payload = json.load(sys.stdin)
with Path(r"{log_path}").open("a", encoding="utf-8") as handle:
handle.write(json.dumps(payload) + "\n")
print(json.dumps({{
"hookSpecificOutput": {{
"hookEventName": "PostToolUse",
"additionalContext": "observed apps file payload"
}}
}}))
"#,
log_path = log_path.display(),
);
let hooks = serde_json::json!({
"hooks": {
"PostToolUse": [{
"matcher": DOCUMENT_EXTRACT_HOOK_MATCHER,
"hooks": [{
"type": "command",
"command": format!("python3 {}", script_path.display()),
"statusMessage": "running apps file post tool use hook",
}]
}]
}
});
fs::write(&script_path, script).context("write post tool use hook script")?;
fs::write(home.join("hooks.json"), hooks.to_string()).context("write hooks.json")?;
Ok(())
}
fn read_post_tool_use_hook_inputs(home: &Path) -> Result<Vec<Value>> {
fs::read_to_string(home.join("post_tool_use_hook_log.jsonl"))
.context("read post tool use hook log")?
.lines()
.filter(|line| !line.trim().is_empty())
.map(|line| serde_json::from_str(line).context("parse post tool use hook input"))
.collect()
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn codex_apps_file_params_upload_local_paths_before_mcp_tool_call() -> Result<()> {
let server = start_mock_server().await;
@@ -101,7 +155,17 @@ async fn codex_apps_file_params_upload_local_paths_before_mcp_tool_call() -> Res
let mut builder = test_codex()
.with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing())
.with_config(move |config| configure_apps(config, apps_server.chatgpt_base_url.as_str()));
.with_pre_build_hook(move |home| {
if let Err(error) = write_post_tool_use_hook(home) {
panic!("failed to write apps file post tool use hook fixture: {error}");
}
})
.with_config(move |config| {
configure_apps(config, apps_server.chatgpt_base_url.as_str());
if let Err(err) = config.features.enable(Feature::CodexHooks) {
panic!("test config should allow feature update: {err}");
}
});
let test = builder.build(&server).await?;
tokio::fs::write(test.cwd.path().join("report.txt"), b"hello world").await?;
@@ -164,6 +228,20 @@ async fn codex_apps_file_params_upload_local_paths_before_mcp_tool_call() -> Res
}))
);
let hook_inputs = read_post_tool_use_hook_inputs(test.codex_home_path())?;
assert_eq!(hook_inputs.len(), 1);
assert_eq!(
hook_inputs[0]["tool_input"]["file"],
json!({
"download_url": format!("{}/download/file_123", server.uri()),
"file_id": "file_123",
"mime_type": "text/plain",
"file_name": "report.txt",
"uri": "sediment://file_123",
"file_size_bytes": 11,
})
);
server.verify().await;
Ok(())
}
@@ -37,7 +37,6 @@ const REQUESTED_MODEL: &str = "gpt-5.4";
const REQUESTED_REASONING_EFFORT: ReasoningEffort = ReasoningEffort::Low;
const ROLE_MODEL: &str = "gpt-5.4";
const ROLE_REASONING_EFFORT: ReasoningEffort = ReasoningEffort::High;
const SPAWNED_AGENT_DEVELOPER_INSTRUCTIONS: &str = "You are a newly spawned agent in a team of agents collaborating to complete a task. You can spawn sub-agents to handle subtasks, and those sub-agents can spawn their own sub-agents. You are responsible for returning the response to your assigned task in the final channel. When you give your response, the contents of your response in the final channel will be immediately delivered back to your parent agent. The prior conversation history was forked from your parent agent. Treat the next user message as your assigned task, and use the forked history only as background context.";
fn body_contains(req: &wiremock::Request, text: &str) -> bool {
let is_zstd = req
@@ -425,7 +424,7 @@ async fn spawn_agent_requested_model_and_reasoning_override_inherited_settings_w
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn spawned_multi_agent_v2_child_receives_xml_tagged_developer_context() -> Result<()> {
async fn spawned_multi_agent_v2_child_inherits_parent_developer_context() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
@@ -446,9 +445,7 @@ async fn spawned_multi_agent_v2_child_receives_xml_tagged_developer_context() ->
let _child_request_log = mount_sse_once_match(
&server,
|req: &wiremock::Request| {
body_contains(req, CHILD_PROMPT) && body_contains(req, "<spawned_agent_context>")
},
|req: &wiremock::Request| body_contains(req, CHILD_PROMPT),
sse(vec![
ev_response_created("resp-child-1"),
ev_completed("resp-child-1"),
@@ -459,9 +456,7 @@ async fn spawned_multi_agent_v2_child_receives_xml_tagged_developer_context() ->
let _turn1_followup = mount_sse_once_match(
&server,
|req: &wiremock::Request| {
body_contains(req, "function_call_output")
&& body_contains(req, "/root/worker")
&& !body_contains(req, "<spawned_agent_context>")
body_contains(req, "function_call_output") && body_contains(req, "/root/worker")
},
sse(vec![
ev_response_created("resp-turn1-2"),
@@ -494,9 +489,7 @@ async fn spawned_multi_agent_v2_child_receives_xml_tagged_developer_context() ->
.unwrap_or_default()
.into_iter()
.find(|request| {
body_contains(request, CHILD_PROMPT)
&& body_contains(request, "<spawned_agent_context>")
&& body_contains(request, SPAWNED_AGENT_DEVELOPER_INSTRUCTIONS)
body_contains(request, CHILD_PROMPT) && !body_contains(request, SPAWN_CALL_ID)
})
{
break request;
@@ -510,11 +503,6 @@ async fn spawned_multi_agent_v2_child_receives_xml_tagged_developer_context() ->
&child_request,
"Parent developer instructions."
));
assert!(body_contains(&child_request, "<spawned_agent_context>"));
assert!(body_contains(
&child_request,
SPAWNED_AGENT_DEVELOPER_INSTRUCTIONS
));
assert!(body_contains(&child_request, CHILD_PROMPT));
Ok(())
@@ -542,9 +530,7 @@ async fn skills_toggle_skips_instructions_for_parent_and_spawned_child() -> Resu
let _child_request_log = mount_sse_once_match(
&server,
|req: &wiremock::Request| {
body_contains(req, CHILD_PROMPT) && body_contains(req, "<spawned_agent_context>")
},
|req: &wiremock::Request| body_contains(req, CHILD_PROMPT),
sse(vec![
ev_response_created("resp-child-1"),
ev_completed("resp-child-1"),
@@ -555,9 +541,7 @@ async fn skills_toggle_skips_instructions_for_parent_and_spawned_child() -> Resu
let _turn1_followup = mount_sse_once_match(
&server,
|req: &wiremock::Request| {
body_contains(req, "function_call_output")
&& body_contains(req, "/root/worker")
&& !body_contains(req, "<spawned_agent_context>")
body_contains(req, "function_call_output") && body_contains(req, "/root/worker")
},
sse(vec![
ev_response_created("resp-turn1-2"),
@@ -599,8 +583,7 @@ async fn skills_toggle_skips_instructions_for_parent_and_spawned_child() -> Resu
.unwrap_or_default()
.into_iter()
.find(|request| {
body_contains(request, CHILD_PROMPT)
&& body_contains(request, "<spawned_agent_context>")
body_contains(request, CHILD_PROMPT) && !body_contains(request, SPAWN_CALL_ID)
})
{
break request;
+4 -1
View File
@@ -323,8 +323,11 @@ impl LocalProcess {
)
};
let has_new_terminal_event =
response.exited && after_seq < response.next_seq.saturating_sub(1);
if !response.chunks.is_empty()
|| response.exited
|| response.closed
|| has_new_terminal_event
|| tokio::time::Instant::now() >= deadline
{
let _total_bytes: usize = response
@@ -7,21 +7,6 @@
"string",
"null"
]
},
"PermissionRequestToolInput": {
"additionalProperties": false,
"properties": {
"command": {
"type": "string"
},
"description": {
"type": "string"
}
},
"required": [
"command"
],
"type": "object"
}
},
"properties": {
@@ -48,9 +33,7 @@
"session_id": {
"type": "string"
},
"tool_input": {
"$ref": "#/definitions/PermissionRequestToolInput"
},
"tool_input": true,
"tool_name": {
"type": "string"
},
@@ -7,18 +7,6 @@
"string",
"null"
]
},
"PostToolUseToolInput": {
"additionalProperties": false,
"properties": {
"command": {
"type": "string"
}
},
"required": [
"command"
],
"type": "object"
}
},
"properties": {
@@ -45,9 +33,7 @@
"session_id": {
"type": "string"
},
"tool_input": {
"$ref": "#/definitions/PostToolUseToolInput"
},
"tool_input": true,
"tool_name": {
"type": "string"
},
@@ -7,18 +7,6 @@
"string",
"null"
]
},
"PreToolUseToolInput": {
"additionalProperties": false,
"properties": {
"command": {
"type": "string"
}
},
"required": [
"command"
],
"type": "object"
}
},
"properties": {
@@ -45,9 +33,7 @@
"session_id": {
"type": "string"
},
"tool_input": {
"$ref": "#/definitions/PreToolUseToolInput"
},
"tool_input": true,
"tool_name": {
"type": "string"
},
+4 -4
View File
@@ -125,7 +125,7 @@ with Path(r"{log_path}").open("a", encoding="utf-8") as handle:
tool_name: "Bash".to_string(),
matcher_aliases: Vec::new(),
tool_use_id: "tool-1".to_string(),
command: "echo hello".to_string(),
tool_input: serde_json::json!({ "command": "echo hello" }),
});
assert_eq!(preview.len(), 1);
assert_eq!(preview[0].source_path, managed_dir);
@@ -141,7 +141,7 @@ with Path(r"{log_path}").open("a", encoding="utf-8") as handle:
tool_name: "Bash".to_string(),
matcher_aliases: Vec::new(),
tool_use_id: "tool-1".to_string(),
command: "echo hello".to_string(),
tool_input: serde_json::json!({ "command": "echo hello" }),
})
.await;
@@ -212,7 +212,7 @@ fn requirements_managed_hooks_warn_when_managed_dir_is_missing() {
tool_name: "Bash".to_string(),
matcher_aliases: Vec::new(),
tool_use_id: "tool-1".to_string(),
command: "echo hello".to_string(),
tool_input: serde_json::json!({ "command": "echo hello" }),
})
.is_empty()
);
@@ -318,7 +318,7 @@ fn discovers_hooks_from_json_and_toml_in_the_same_layer() {
tool_name: "Bash".to_string(),
matcher_aliases: Vec::new(),
tool_use_id: "tool-1".to_string(),
command: "echo hello".to_string(),
tool_input: serde_json::json!({ "command": "echo hello" }),
});
assert_eq!(preview.len(), 2);
assert!(engine.handlers.iter().all(|handler| !handler.is_managed));
+26 -9
View File
@@ -183,7 +183,7 @@ mod tests {
}
#[test]
fn matcher_uses_regex_matching() {
fn exact_matcher_supports_pipe_alternatives() {
assert!(matches_matcher(Some("Edit|Write"), Some("Edit")));
assert!(matches_matcher(Some("Edit|Write"), Some("Write")));
assert!(!matches_matcher(Some("Edit|Write"), Some("Bash")));
@@ -191,23 +191,40 @@ mod tests {
}
#[test]
fn matcher_exact_string_does_not_substring_match() {
fn literal_matcher_uses_exact_matching() {
assert!(matches_matcher(Some("Bash"), Some("Bash")));
assert!(!matches_matcher(Some("Bash"), Some("BashOutput")));
assert_eq!(validate_matcher_pattern("Bash"), Ok(()));
}
#[test]
fn matcher_uses_regex_when_it_contains_other_characters() {
assert!(matches_matcher(Some("^Bash"), Some("BashOutput")));
assert!(matches_matcher(
Some("mcp__memory__.*"),
Some("mcp__memory__create_entities"),
Some("mcp__memory__create_entities")
));
assert!(!matches_matcher(
Some("mcp__memory"),
Some("mcp__memory__create_entities")
));
assert_eq!(validate_matcher_pattern("mcp__memory"), Ok(()));
}
#[test]
fn matcher_uses_regex_when_it_contains_regex_characters() {
assert!(matches_matcher(Some("^Bash"), Some("BashOutput")));
assert_eq!(validate_matcher_pattern("^Bash"), Ok(()));
}
#[test]
fn mcp_matchers_support_regex_wildcards() {
assert!(matches_matcher(
Some("mcp__memory__.*"),
Some("mcp__memory__create_entities")
));
assert!(matches_matcher(
Some("mcp__.*__write.*"),
Some("mcp__filesystem__write_file")
));
assert!(!matches_matcher(
Some("mcp__.*__write.*"),
Some("mcp__filesystem__read_file")
));
assert_eq!(validate_matcher_pattern("mcp__memory__.*"), Ok(()));
}
@@ -22,7 +22,6 @@ use crate::engine::command_runner::CommandRunResult;
use crate::engine::dispatcher;
use crate::engine::output_parser;
use crate::schema::PermissionRequestCommandInput;
use crate::schema::PermissionRequestToolInput;
use codex_protocol::ThreadId;
use codex_protocol::protocol::HookCompletedEvent;
use codex_protocol::protocol::HookEventName;
@@ -30,6 +29,7 @@ use codex_protocol::protocol::HookOutputEntry;
use codex_protocol::protocol::HookOutputEntryKind;
use codex_protocol::protocol::HookRunStatus;
use codex_protocol::protocol::HookRunSummary;
use serde_json::Value;
#[derive(Debug, Clone)]
pub struct PermissionRequestRequest {
@@ -42,8 +42,7 @@ pub struct PermissionRequestRequest {
pub tool_name: String,
pub matcher_aliases: Vec<String>,
pub run_id_suffix: String,
pub command: String,
pub description: Option<String>,
pub tool_input: Value,
}
#[derive(Debug, Clone, PartialEq, Eq)]
@@ -178,10 +177,7 @@ fn build_command_input(request: &PermissionRequestRequest) -> PermissionRequestC
model: request.model.clone(),
permission_mode: request.permission_mode.clone(),
tool_name: request.tool_name.clone(),
tool_input: PermissionRequestToolInput {
command: request.command.clone(),
description: request.description.clone(),
},
tool_input: request.tool_input.clone(),
}
}
+5 -7
View File
@@ -17,7 +17,6 @@ use crate::engine::command_runner::CommandRunResult;
use crate::engine::dispatcher;
use crate::engine::output_parser;
use crate::schema::PostToolUseCommandInput;
use crate::schema::PostToolUseToolInput;
#[derive(Debug, Clone)]
pub struct PostToolUseRequest {
@@ -30,7 +29,7 @@ pub struct PostToolUseRequest {
pub tool_name: String,
pub matcher_aliases: Vec<String>,
pub tool_use_id: String,
pub command: String,
pub tool_input: Value,
pub tool_response: Value,
}
@@ -146,7 +145,8 @@ pub(crate) async fn run(
///
/// Handler selection may include internal matcher aliases, but hook stdin keeps
/// the canonical `tool_name` for logs and for consumers that pair pre/post
/// events across processes.
/// events across processes. Shell-like tools pass `{ "command": ... }` as
/// `tool_input`; MCP tools pass their resolved JSON arguments.
fn command_input_json(request: &PostToolUseRequest) -> Result<String, serde_json::Error> {
serde_json::to_string(&PostToolUseCommandInput {
session_id: request.session_id.to_string(),
@@ -157,9 +157,7 @@ fn command_input_json(request: &PostToolUseRequest) -> Result<String, serde_json
model: request.model.clone(),
permission_mode: request.permission_mode.clone(),
tool_name: request.tool_name.clone(),
tool_input: PostToolUseToolInput {
command: request.command.clone(),
},
tool_input: request.tool_input.clone(),
tool_response: request.tool_response.clone(),
tool_use_id: request.tool_use_id.clone(),
})
@@ -579,7 +577,7 @@ mod tests {
tool_name: "Bash".to_string(),
matcher_aliases: Vec::new(),
tool_use_id: tool_use_id.to_string(),
command: "echo hello".to_string(),
tool_input: json!({ "command": "echo hello" }),
tool_response: json!({"ok": true}),
}
}
+6 -6
View File
@@ -8,6 +8,7 @@ use codex_protocol::protocol::HookOutputEntryKind;
use codex_protocol::protocol::HookRunStatus;
use codex_protocol::protocol::HookRunSummary;
use codex_utils_absolute_path::AbsolutePathBuf;
use serde_json::Value;
use super::common;
use crate::engine::CommandShell;
@@ -28,7 +29,7 @@ pub struct PreToolUseRequest {
pub tool_name: String,
pub matcher_aliases: Vec<String>,
pub tool_use_id: String,
pub command: String,
pub tool_input: Value,
}
#[derive(Debug)]
@@ -124,7 +125,8 @@ pub(crate) async fn run(
///
/// Handler selection may include internal matcher aliases, but hook stdin keeps
/// the canonical `tool_name` so audit logs and downstream policy decisions stay
/// stable.
/// stable. Shell-like tools pass `{ "command": ... }` as `tool_input`; MCP
/// tools pass their resolved JSON arguments.
fn command_input_json(request: &PreToolUseRequest) -> Result<String, serde_json::Error> {
serde_json::to_string(&PreToolUseCommandInput {
session_id: request.session_id.to_string(),
@@ -135,9 +137,7 @@ fn command_input_json(request: &PreToolUseRequest) -> Result<String, serde_json:
model: request.model.clone(),
permission_mode: request.permission_mode.clone(),
tool_name: request.tool_name.clone(),
tool_input: crate::schema::PreToolUseToolInput {
command: request.command.clone(),
},
tool_input: request.tool_input.clone(),
tool_use_id: request.tool_use_id.clone(),
})
}
@@ -568,7 +568,7 @@ mod tests {
tool_name: "Bash".to_string(),
matcher_aliases: Vec::new(),
tool_use_id: tool_use_id.to_string(),
command: "echo hello".to_string(),
tool_input: serde_json::json!({ "command": "echo hello" }),
}
}
}
+3 -26
View File
@@ -210,13 +210,6 @@ pub(crate) enum PreToolUseDecisionWire {
Block,
}
#[derive(Debug, Clone, Serialize, JsonSchema)]
#[serde(rename_all = "camelCase")]
#[serde(deny_unknown_fields)]
pub(crate) struct PreToolUseToolInput {
pub command: String,
}
#[derive(Debug, Clone, Serialize, JsonSchema)]
#[serde(deny_unknown_fields)]
#[schemars(rename = "pre-tool-use.command.input")]
@@ -232,19 +225,10 @@ pub(crate) struct PreToolUseCommandInput {
#[schemars(schema_with = "permission_mode_schema")]
pub permission_mode: String,
pub tool_name: String,
pub tool_input: PreToolUseToolInput,
pub tool_input: Value,
pub tool_use_id: String,
}
#[derive(Debug, Clone, Serialize, JsonSchema)]
#[serde(rename_all = "camelCase")]
#[serde(deny_unknown_fields)]
pub(crate) struct PermissionRequestToolInput {
pub command: String,
#[serde(skip_serializing_if = "Option::is_none")]
pub description: Option<String>,
}
#[derive(Debug, Clone, Serialize, JsonSchema)]
#[serde(deny_unknown_fields)]
#[schemars(rename = "permission-request.command.input")]
@@ -260,14 +244,7 @@ pub(crate) struct PermissionRequestCommandInput {
#[schemars(schema_with = "permission_mode_schema")]
pub permission_mode: String,
pub tool_name: String,
pub tool_input: PermissionRequestToolInput,
}
#[derive(Debug, Clone, Serialize, JsonSchema)]
#[serde(rename_all = "camelCase")]
#[serde(deny_unknown_fields)]
pub(crate) struct PostToolUseToolInput {
pub command: String,
pub tool_input: Value,
}
#[derive(Debug, Clone, Serialize, JsonSchema)]
@@ -285,7 +262,7 @@ pub(crate) struct PostToolUseCommandInput {
#[schemars(schema_with = "permission_mode_schema")]
pub permission_mode: String,
pub tool_name: String,
pub tool_input: PostToolUseToolInput,
pub tool_input: Value,
pub tool_response: Value,
pub tool_use_id: String,
}
+3 -3
View File
@@ -87,8 +87,8 @@ pub enum ApprovalsReviewer {
#[default]
#[serde(rename = "user")]
User,
#[serde(rename = "auto_review", alias = "guardian_subagent")]
#[strum(serialize = "auto_review")]
#[serde(rename = "guardian_subagent", alias = "auto_review")]
#[strum(serialize = "guardian_subagent")]
AutoReview,
}
@@ -609,7 +609,7 @@ mod tests {
);
assert_eq!(
serde_json::to_string(&ApprovalsReviewer::AutoReview).expect("serialize reviewer"),
"\"auto_review\""
"\"guardian_subagent\""
);
for value in ["user", "auto_review", "guardian_subagent"] {
+3 -3
View File
@@ -1678,7 +1678,7 @@ async fn update_feature_flags_enabling_guardian_selects_auto_review() -> Result<
let config = std::fs::read_to_string(codex_home.path().join("config.toml"))?;
assert!(config.contains("guardian_approval = true"));
assert!(config.contains("approvals_reviewer = \"auto_review\""));
assert!(config.contains("approvals_reviewer = \"guardian_subagent\""));
assert!(config.contains("approval_policy = \"on-request\""));
assert!(config.contains("sandbox_mode = \"workspace-write\""));
Ok(())
@@ -1838,7 +1838,7 @@ async fn update_feature_flags_enabling_guardian_overrides_explicit_manual_review
);
let config = std::fs::read_to_string(codex_home.path().join("config.toml"))?;
assert!(config.contains("approvals_reviewer = \"auto_review\""));
assert!(config.contains("approvals_reviewer = \"guardian_subagent\""));
assert!(config.contains("guardian_approval = true"));
assert!(config.contains("approval_policy = \"on-request\""));
assert!(config.contains("sandbox_mode = \"workspace-write\""));
@@ -1972,7 +1972,7 @@ async fn update_feature_flags_enabling_guardian_in_profile_sets_profile_auto_rev
);
assert_eq!(
profile_config.get("approvals_reviewer"),
Some(&TomlValue::String("auto_review".to_string()))
Some(&TomlValue::String("guardian_subagent".to_string()))
);
Ok(())
}
+1 -1
View File
@@ -259,7 +259,7 @@ const MULTI_AGENT_ENABLE_TITLE: &str = "Enable subagents?";
const MULTI_AGENT_ENABLE_YES: &str = "Yes, enable";
const MULTI_AGENT_ENABLE_NO: &str = "Not now";
const MULTI_AGENT_ENABLE_NOTICE: &str = "Subagents will be enabled in the next session.";
const TRUSTED_ACCESS_FOR_CYBER_VERIFICATION_WARNING: &str = "Your account was flagged for potentially high-risk cyber activity. Requests may be slower while additional verification is applied. To regain faster access, apply for trusted access: https://chatgpt.com/cyber or learn more: https://developers.openai.com/codex/concepts/cyber-safety";
const TRUSTED_ACCESS_FOR_CYBER_VERIFICATION_WARNING: &str = "Your conversations have multiple flags for possible cybersecurity risk. Responses may take longer because extra safety checks are on. To get authorized for security work, join the Trusted Access for Cyber program: https://chatgpt.com/cyber";
const MEMORIES_DOC_URL: &str = "https://developers.openai.com/codex/memories";
const MEMORIES_ENABLE_TITLE: &str = "Enable memories?";
const MEMORIES_ENABLE_YES: &str = "Yes, enable";
@@ -738,8 +738,9 @@ async fn live_app_server_model_verification_renders_warning() {
let cells = drain_insert_history(&mut rx);
assert_eq!(cells.len(), 1);
let rendered = lines_to_single_string(&cells[0]);
assert!(rendered.contains("flagged for potentially high-risk cyber activity"));
assert!(rendered.contains("slower while additional verification"));
assert!(rendered.contains("multiple flags for possible cybersecurity risk"));
assert!(rendered.contains("extra safety checks are on"));
assert!(rendered.contains("Trusted Access for Cyber"));
assert!(rendered.contains("https://chatgpt.com/cyber"));
}
@@ -62,8 +62,9 @@ async fn core_model_verification_renders_warning() {
let cells = drain_insert_history(&mut rx);
assert_eq!(cells.len(), 1);
let rendered = lines_to_single_string(&cells[0]);
assert!(rendered.contains("flagged for potentially high-risk cyber activity"));
assert!(rendered.contains("slower while additional verification"));
assert!(rendered.contains("multiple flags for possible cybersecurity risk"));
assert!(rendered.contains("extra safety checks are on"));
assert!(rendered.contains("Trusted Access for Cyber"));
assert!(rendered.contains("https://chatgpt.com/cyber"));
}
+2 -2
View File
@@ -724,7 +724,7 @@ mod tests {
rendered.contains("allowed_approval_policies: on-request (source: cloud requirements)")
);
assert!(rendered.contains(
"allowed_approvals_reviewers: auto_review (source: MDM managed_config.toml (legacy))"
"allowed_approvals_reviewers: guardian_subagent (source: MDM managed_config.toml (legacy))"
));
assert!(
rendered.contains(
@@ -779,7 +779,7 @@ mod tests {
let rendered = render_to_text(&render_debug_config_lines(&stack));
assert!(rendered.contains(
"allowed_approvals_reviewers: auto_review (source: MDM managed_config.toml (legacy))"
"allowed_approvals_reviewers: guardian_subagent (source: MDM managed_config.toml (legacy))"
));
assert!(!rendered.contains("Requirements:\n <none>"));
}
@@ -146,6 +146,7 @@ class AppToolsConfig(BaseModel):
class ApprovalsReviewer(Enum):
user = "user"
auto_review = "auto_review"
guardian_subagent = "guardian_subagent"
+37 -2
View File
@@ -4,7 +4,12 @@ from pathlib import Path
from typing import Any
from codex_app_server.client import AppServerClient, _params_dict
from codex_app_server.generated.v2_all import ThreadListParams, ThreadTokenUsageUpdatedNotification
from codex_app_server.generated.v2_all import (
ApprovalsReviewer,
ThreadListParams,
ThreadResumeResponse,
ThreadTokenUsageUpdatedNotification,
)
from codex_app_server.models import UnknownNotification
ROOT = Path(__file__).resolve().parents[1]
@@ -40,6 +45,34 @@ def test_generated_v2_bundle_has_single_shared_plan_type_definition() -> None:
assert source.count("class PlanType(") == 1
def test_thread_resume_response_accepts_auto_review_reviewer() -> None:
response = ThreadResumeResponse.model_validate(
{
"approvalPolicy": "on-request",
"approvalsReviewer": "auto_review",
"cwd": "/tmp",
"model": "gpt-5",
"modelProvider": "openai",
"sandbox": {"type": "dangerFullAccess"},
"thread": {
"cliVersion": "1.0.0",
"createdAt": 1,
"cwd": "/tmp",
"ephemeral": False,
"id": "thread-1",
"modelProvider": "openai",
"preview": "",
"source": "cli",
"status": {"type": "idle"},
"turns": [],
"updatedAt": 1,
},
}
)
assert response.approvals_reviewer is ApprovalsReviewer.auto_review
def test_notifications_are_typed_with_canonical_v2_methods() -> None:
client = AppServerClient()
event = client._coerce_notification(
@@ -89,7 +122,9 @@ def test_unknown_notifications_fall_back_to_unknown_payloads() -> None:
def test_invalid_notification_payload_falls_back_to_unknown() -> None:
client = AppServerClient()
event = client._coerce_notification("thread/tokenUsage/updated", {"threadId": "missing"})
event = client._coerce_notification(
"thread/tokenUsage/updated", {"threadId": "missing"}
)
assert event.method == "thread/tokenUsage/updated"
assert isinstance(event.payload, UnknownNotification)