mirror of
https://github.com/openai/codex.git
synced 2026-05-25 21:45:22 +00:00
10cf1f79dd
## Why - Similar change as https://github.com/openai/codex/pull/21219 - Without change: MCP tool calls receive `_meta["x-codex-turn-metadata"]` with various key values. - Issue: MCP servers currently do not know if user input was requested during the turn (Ex: Model decides to prompt the user for approval mid-turn before making a possibly risky tool call). MCP servers may want to know this when tracking latency metrics because these instances are inflated. ## What Changed - With change: MCP turn metadata now includes `user_input_requested_during_turn` when a model-visible `request_user_input` call happened earlier in the turn, propagated in `_meta["x-codex-turn-metadata"]`. - `mark_turn_user_input_requested()` is called when user input is requested through either MCP elicitation (`mcp.rs`) or the `request_user_input` tool (`mod.rs`). - MCP tool call `_meta` is now built immediately before execution (`mcp_tool_call.rs`) so user input requested earlier in the same turn, including within the same tool call via elicitation, is reflected in the metadata. - Normal `/responses` turn metadata headers are unchanged. ## Verification - `codex-rs/core/src/session/mcp_tests.rs` - `codex-rs/core/src/tools/handlers/request_user_input_tests.rs` - `codex-rs/core/src/turn_metadata_tests.rs` - `codex-rs/core/tests/suite/search_tool.rs`
351 lines
12 KiB
Rust
351 lines
12 KiB
Rust
use super::*;
|
|
|
|
use crate::sandbox_tags::sandbox_tag;
|
|
use codex_protocol::models::PermissionProfile;
|
|
use codex_protocol::openai_models::ReasoningEffort as ReasoningEffortConfig;
|
|
use codex_protocol::protocol::SandboxPolicy;
|
|
use codex_protocol::protocol::ThreadSource;
|
|
use core_test_support::PathBufExt;
|
|
use core_test_support::PathExt;
|
|
use pretty_assertions::assert_eq;
|
|
use serde_json::Value;
|
|
use std::collections::HashMap;
|
|
use tempfile::TempDir;
|
|
use tokio::process::Command;
|
|
|
|
fn test_mcp_turn_metadata_context() -> McpTurnMetadataContext<'static> {
|
|
McpTurnMetadataContext {
|
|
model: "gpt-5.4",
|
|
reasoning_effort: Some(ReasoningEffortConfig::High),
|
|
}
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn build_turn_metadata_header_includes_has_changes_for_clean_repo() {
|
|
let temp_dir = TempDir::new().expect("temp dir");
|
|
let repo_path = temp_dir.path().join("repo-東京").abs();
|
|
std::fs::create_dir_all(&repo_path).expect("create repo");
|
|
|
|
Command::new("git")
|
|
.args(["init"])
|
|
.current_dir(&repo_path)
|
|
.output()
|
|
.await
|
|
.expect("git init");
|
|
Command::new("git")
|
|
.args(["config", "user.name", "Test User"])
|
|
.current_dir(&repo_path)
|
|
.output()
|
|
.await
|
|
.expect("git config user.name");
|
|
Command::new("git")
|
|
.args(["config", "user.email", "test@example.com"])
|
|
.current_dir(&repo_path)
|
|
.output()
|
|
.await
|
|
.expect("git config user.email");
|
|
|
|
std::fs::write(repo_path.join("README.md"), "hello").expect("write file");
|
|
Command::new("git")
|
|
.args(["add", "."])
|
|
.current_dir(&repo_path)
|
|
.output()
|
|
.await
|
|
.expect("git add");
|
|
Command::new("git")
|
|
.args(["commit", "-m", "initial"])
|
|
.current_dir(&repo_path)
|
|
.output()
|
|
.await
|
|
.expect("git commit");
|
|
|
|
let header = build_turn_metadata_header(&repo_path, Some("none"))
|
|
.await
|
|
.expect("header");
|
|
assert!(header.is_ascii());
|
|
assert!(!header.contains("東京"));
|
|
let parsed: Value = serde_json::from_str(&header).expect("valid json");
|
|
let expected_repo_path = repo_path.to_string_lossy().into_owned();
|
|
let actual_repo_path = parsed
|
|
.get("workspaces")
|
|
.and_then(Value::as_object)
|
|
.and_then(|workspaces| workspaces.keys().next())
|
|
.expect("workspace path");
|
|
assert_eq!(actual_repo_path, &expected_repo_path);
|
|
let workspace = parsed
|
|
.get("workspaces")
|
|
.and_then(Value::as_object)
|
|
.and_then(|workspaces| workspaces.values().next())
|
|
.cloned()
|
|
.expect("workspace");
|
|
|
|
assert_eq!(
|
|
workspace.get("has_changes").and_then(Value::as_bool),
|
|
Some(false)
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn turn_metadata_state_uses_platform_sandbox_tag() {
|
|
let temp_dir = TempDir::new().expect("temp dir");
|
|
let cwd = temp_dir.path().abs();
|
|
let sandbox_policy = SandboxPolicy::new_read_only_policy();
|
|
let permission_profile = PermissionProfile::read_only();
|
|
|
|
let state = TurnMetadataState::new(
|
|
"session-a".to_string(),
|
|
"thread-a".to_string(),
|
|
Some(ThreadSource::User),
|
|
"turn-a".to_string(),
|
|
cwd,
|
|
&permission_profile,
|
|
WindowsSandboxLevel::Disabled,
|
|
/*enforce_managed_network*/ false,
|
|
);
|
|
|
|
let header = state.current_header_value().expect("header");
|
|
let json: Value = serde_json::from_str(&header).expect("json");
|
|
let sandbox_name = json.get("sandbox").and_then(Value::as_str);
|
|
let session_id = json.get("session_id").and_then(Value::as_str);
|
|
let thread_id = json.get("thread_id").and_then(Value::as_str);
|
|
let thread_source = json.get("thread_source").and_then(Value::as_str);
|
|
|
|
let expected_sandbox = sandbox_tag(&sandbox_policy, WindowsSandboxLevel::Disabled);
|
|
assert_eq!(sandbox_name, Some(expected_sandbox));
|
|
assert_eq!(session_id, Some("session-a"));
|
|
assert_eq!(thread_id, Some("thread-a"));
|
|
assert_eq!(thread_source, Some("user"));
|
|
assert!(json.get("session_source").is_none());
|
|
}
|
|
|
|
#[test]
|
|
fn turn_metadata_state_uses_explicit_subagent_thread_source() {
|
|
let temp_dir = TempDir::new().expect("temp dir");
|
|
let cwd = temp_dir.path().abs();
|
|
let permission_profile = PermissionProfile::read_only();
|
|
let state = TurnMetadataState::new(
|
|
"session-a".to_string(),
|
|
"thread-a".to_string(),
|
|
Some(ThreadSource::Subagent),
|
|
"turn-a".to_string(),
|
|
cwd,
|
|
&permission_profile,
|
|
WindowsSandboxLevel::Disabled,
|
|
/*enforce_managed_network*/ false,
|
|
);
|
|
|
|
let header = state.current_header_value().expect("header");
|
|
let json: Value = serde_json::from_str(&header).expect("json");
|
|
|
|
assert_eq!(json["thread_source"].as_str(), Some("subagent"));
|
|
assert!(json.get("session_source").is_none());
|
|
}
|
|
|
|
#[test]
|
|
fn turn_metadata_state_includes_turn_started_at_unix_ms_after_start() {
|
|
let temp_dir = TempDir::new().expect("temp dir");
|
|
let cwd = temp_dir.path().abs();
|
|
let permission_profile = PermissionProfile::read_only();
|
|
|
|
let state = TurnMetadataState::new(
|
|
"session-a".to_string(),
|
|
"thread-a".to_string(),
|
|
Some(ThreadSource::User),
|
|
"turn-a".to_string(),
|
|
cwd,
|
|
&permission_profile,
|
|
WindowsSandboxLevel::Disabled,
|
|
/*enforce_managed_network*/ false,
|
|
);
|
|
state.set_turn_started_at_unix_ms(/*turn_started_at_unix_ms*/ 1_700_000_000_123);
|
|
|
|
let header = state.current_header_value().expect("header");
|
|
let json: Value = serde_json::from_str(&header).expect("json");
|
|
|
|
assert_eq!(
|
|
json["turn_started_at_unix_ms"].as_i64(),
|
|
Some(1_700_000_000_123)
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn turn_metadata_state_includes_model_and_reasoning_effort_only_in_request_meta() {
|
|
let temp_dir = TempDir::new().expect("temp dir");
|
|
let cwd = temp_dir.path().abs();
|
|
let permission_profile = PermissionProfile::read_only();
|
|
|
|
let state = TurnMetadataState::new(
|
|
"session-a".to_string(),
|
|
"thread-a".to_string(),
|
|
/*thread_source*/ None,
|
|
"turn-a".to_string(),
|
|
cwd,
|
|
&permission_profile,
|
|
WindowsSandboxLevel::Disabled,
|
|
/*enforce_managed_network*/ false,
|
|
);
|
|
|
|
let header = state.current_header_value().expect("header");
|
|
let header_json: Value = serde_json::from_str(&header).expect("json");
|
|
assert!(header_json.get("model").is_none());
|
|
assert!(header_json.get("reasoning_effort").is_none());
|
|
|
|
let meta = state
|
|
.current_meta_value_for_mcp_request(test_mcp_turn_metadata_context())
|
|
.expect("turn metadata should be present");
|
|
assert_eq!(meta["model"].as_str(), Some("gpt-5.4"));
|
|
assert_eq!(meta["reasoning_effort"].as_str(), Some("high"));
|
|
|
|
let meta_without_reasoning_effort = state
|
|
.current_meta_value_for_mcp_request(McpTurnMetadataContext {
|
|
model: "gpt-5.4",
|
|
reasoning_effort: None,
|
|
})
|
|
.expect("turn metadata should be present");
|
|
assert_eq!(
|
|
meta_without_reasoning_effort["model"].as_str(),
|
|
Some("gpt-5.4")
|
|
);
|
|
assert!(
|
|
meta_without_reasoning_effort
|
|
.get("reasoning_effort")
|
|
.is_none()
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn turn_metadata_state_marks_user_input_requested_during_turn_only_for_mcp_request_meta() {
|
|
let temp_dir = TempDir::new().expect("temp dir");
|
|
let cwd = temp_dir.path().abs();
|
|
let permission_profile = PermissionProfile::read_only();
|
|
|
|
let state = TurnMetadataState::new(
|
|
"session-a".to_string(),
|
|
"thread-a".to_string(),
|
|
/*thread_source*/ None,
|
|
"turn-a".to_string(),
|
|
cwd,
|
|
&permission_profile,
|
|
WindowsSandboxLevel::Disabled,
|
|
/*enforce_managed_network*/ false,
|
|
);
|
|
|
|
let header = state.current_header_value().expect("header");
|
|
let header_json: Value = serde_json::from_str(&header).expect("json");
|
|
assert!(
|
|
header_json
|
|
.get(USER_INPUT_REQUESTED_DURING_TURN_KEY)
|
|
.is_none()
|
|
);
|
|
|
|
let meta = state
|
|
.current_meta_value_for_mcp_request(test_mcp_turn_metadata_context())
|
|
.expect("turn metadata should be present");
|
|
assert!(meta.get(USER_INPUT_REQUESTED_DURING_TURN_KEY).is_none());
|
|
|
|
state.mark_user_input_requested_during_turn();
|
|
|
|
let header = state.current_header_value().expect("header");
|
|
let header_json: Value = serde_json::from_str(&header).expect("json");
|
|
assert!(
|
|
header_json
|
|
.get(USER_INPUT_REQUESTED_DURING_TURN_KEY)
|
|
.is_none()
|
|
);
|
|
|
|
let meta = state
|
|
.current_meta_value_for_mcp_request(test_mcp_turn_metadata_context())
|
|
.expect("turn metadata should be present");
|
|
assert_eq!(
|
|
meta.get(USER_INPUT_REQUESTED_DURING_TURN_KEY)
|
|
.and_then(Value::as_bool),
|
|
Some(true)
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn turn_metadata_state_ignores_client_turn_started_at_unix_ms_before_start() {
|
|
let temp_dir = TempDir::new().expect("temp dir");
|
|
let cwd = temp_dir.path().abs();
|
|
let permission_profile = PermissionProfile::read_only();
|
|
|
|
let state = TurnMetadataState::new(
|
|
"session-a".to_string(),
|
|
"thread-a".to_string(),
|
|
Some(ThreadSource::User),
|
|
"turn-a".to_string(),
|
|
cwd,
|
|
&permission_profile,
|
|
WindowsSandboxLevel::Disabled,
|
|
/*enforce_managed_network*/ false,
|
|
);
|
|
state.set_responsesapi_client_metadata(HashMap::from([(
|
|
"turn_started_at_unix_ms".to_string(),
|
|
"client-supplied".to_string(),
|
|
)]));
|
|
|
|
let header = state.current_header_value().expect("header");
|
|
let json: Value = serde_json::from_str(&header).expect("json");
|
|
|
|
assert!(json.get("turn_started_at_unix_ms").is_none());
|
|
}
|
|
|
|
#[test]
|
|
fn turn_metadata_state_merges_client_metadata_without_replacing_reserved_fields() {
|
|
let temp_dir = TempDir::new().expect("temp dir");
|
|
let cwd = temp_dir.path().abs();
|
|
let permission_profile = PermissionProfile::read_only();
|
|
|
|
let state = TurnMetadataState::new(
|
|
"session-a".to_string(),
|
|
"thread-a".to_string(),
|
|
Some(ThreadSource::User),
|
|
"turn-a".to_string(),
|
|
cwd,
|
|
&permission_profile,
|
|
WindowsSandboxLevel::Disabled,
|
|
/*enforce_managed_network*/ false,
|
|
);
|
|
state.set_responsesapi_client_metadata(HashMap::from([
|
|
("fiber_run_id".to_string(), "fiber-123".to_string()),
|
|
("origin".to_string(), "東京".to_string()),
|
|
("model".to_string(), "client-supplied".to_string()),
|
|
(
|
|
"reasoning_effort".to_string(),
|
|
"client-supplied".to_string(),
|
|
),
|
|
("session_id".to_string(), "client-supplied".to_string()),
|
|
("thread_id".to_string(), "client-supplied".to_string()),
|
|
("thread_source".to_string(), "client-supplied".to_string()),
|
|
(
|
|
"turn_started_at_unix_ms".to_string(),
|
|
"client-supplied".to_string(),
|
|
),
|
|
]));
|
|
state.set_turn_started_at_unix_ms(/*turn_started_at_unix_ms*/ 1_700_000_000_123);
|
|
|
|
let header = state.current_header_value().expect("header");
|
|
assert!(header.is_ascii());
|
|
assert!(!header.contains("東京"));
|
|
let json: Value = serde_json::from_str(&header).expect("json");
|
|
|
|
assert_eq!(json["fiber_run_id"].as_str(), Some("fiber-123"));
|
|
assert_eq!(json["origin"].as_str(), Some("東京"));
|
|
assert_eq!(json["model"].as_str(), Some("client-supplied"));
|
|
assert_eq!(json["reasoning_effort"].as_str(), Some("client-supplied"));
|
|
assert_eq!(json["session_id"].as_str(), Some("session-a"));
|
|
assert_eq!(json["thread_id"].as_str(), Some("thread-a"));
|
|
assert_eq!(json["thread_source"].as_str(), Some("user"));
|
|
assert_eq!(json["turn_id"].as_str(), Some("turn-a"));
|
|
assert_eq!(
|
|
json["turn_started_at_unix_ms"].as_i64(),
|
|
Some(1_700_000_000_123)
|
|
);
|
|
|
|
let meta = state
|
|
.current_meta_value_for_mcp_request(test_mcp_turn_metadata_context())
|
|
.expect("turn metadata should be present");
|
|
assert_eq!(meta["model"].as_str(), Some("gpt-5.4"));
|
|
assert_eq!(meta["reasoning_effort"].as_str(), Some("high"));
|
|
}
|