mirror of
https://github.com/openai/codex.git
synced 2026-05-21 19:45:26 +00:00
Narrow app-server runtime capability follow-up
This commit is contained in:
@@ -388,7 +388,6 @@ impl MessageProcessor {
|
||||
Arc::clone(&thread_manager),
|
||||
outgoing.clone(),
|
||||
config_manager.clone(),
|
||||
Arc::clone(&runtime_capabilities),
|
||||
);
|
||||
let plugin_processor = PluginRequestProcessor::new(
|
||||
auth_manager.clone(),
|
||||
|
||||
@@ -8,7 +8,6 @@ pub(crate) struct McpRequestProcessor {
|
||||
thread_manager: Arc<ThreadManager>,
|
||||
outgoing: Arc<OutgoingMessageSender>,
|
||||
config_manager: ConfigManager,
|
||||
runtime_capabilities: Arc<RuntimeCapabilities>,
|
||||
}
|
||||
|
||||
impl McpRequestProcessor {
|
||||
@@ -17,14 +16,12 @@ impl McpRequestProcessor {
|
||||
thread_manager: Arc<ThreadManager>,
|
||||
outgoing: Arc<OutgoingMessageSender>,
|
||||
config_manager: ConfigManager,
|
||||
runtime_capabilities: Arc<RuntimeCapabilities>,
|
||||
) -> Self {
|
||||
Self {
|
||||
auth_manager,
|
||||
thread_manager,
|
||||
outgoing,
|
||||
config_manager,
|
||||
runtime_capabilities,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -207,12 +204,18 @@ impl McpRequestProcessor {
|
||||
.to_mcp_config(self.thread_manager.plugins_manager().as_ref())
|
||||
.await;
|
||||
let auth = self.auth_manager.auth().await;
|
||||
let runtime_environment = runtime_environment_without_thread(
|
||||
self.thread_manager.environment_manager().as_ref(),
|
||||
&self.runtime_capabilities,
|
||||
config.cwd.to_path_buf(),
|
||||
"list MCP server status without thread",
|
||||
)?;
|
||||
let environment_manager = self.thread_manager.environment_manager();
|
||||
let runtime_environment = match environment_manager.default_environment() {
|
||||
Some(environment) => {
|
||||
// Status listing has no turn cwd. This fallback is used only
|
||||
// by executor-backed stdio MCPs whose config omits `cwd`.
|
||||
McpRuntimeEnvironment::new(environment, config.cwd.to_path_buf())
|
||||
}
|
||||
None => McpRuntimeEnvironment::new(
|
||||
environment_manager.local_environment(),
|
||||
config.cwd.to_path_buf(),
|
||||
),
|
||||
};
|
||||
|
||||
tokio::spawn(async move {
|
||||
Self::list_mcp_server_status_task(
|
||||
@@ -366,12 +369,15 @@ impl McpRequestProcessor {
|
||||
.to_mcp_config(self.thread_manager.plugins_manager().as_ref())
|
||||
.await;
|
||||
let auth = self.auth_manager.auth().await;
|
||||
let runtime_environment = runtime_environment_without_thread(
|
||||
self.thread_manager.environment_manager().as_ref(),
|
||||
&self.runtime_capabilities,
|
||||
config.cwd.to_path_buf(),
|
||||
"read MCP resource without thread",
|
||||
)?;
|
||||
let runtime_environment = {
|
||||
let environment_manager = self.thread_manager.environment_manager();
|
||||
let environment = environment_manager
|
||||
.default_environment()
|
||||
.unwrap_or_else(|| environment_manager.local_environment());
|
||||
// Resource reads without a thread have no turn cwd. This fallback
|
||||
// is used only by executor-backed stdio MCPs whose config omits `cwd`.
|
||||
McpRuntimeEnvironment::new(environment, config.cwd.to_path_buf())
|
||||
};
|
||||
let request_id = request_id.clone();
|
||||
|
||||
tokio::spawn(async move {
|
||||
@@ -429,64 +435,6 @@ impl McpRequestProcessor {
|
||||
}
|
||||
}
|
||||
|
||||
fn runtime_environment_without_thread(
|
||||
environment_manager: &EnvironmentManager,
|
||||
runtime_capabilities: &RuntimeCapabilities,
|
||||
cwd: PathBuf,
|
||||
local_fallback_operation: &str,
|
||||
) -> Result<McpRuntimeEnvironment, JSONRPCErrorError> {
|
||||
let environment = match environment_manager.default_environment() {
|
||||
Some(environment) => environment,
|
||||
None => runtime_capabilities
|
||||
.require_local_environment(local_fallback_operation)
|
||||
.map_err(|err| internal_error(err.to_string()))?,
|
||||
};
|
||||
// Threadless MCP requests have no turn cwd. This fallback is used only
|
||||
// by executor-backed stdio MCPs whose config omits `cwd`.
|
||||
Ok(McpRuntimeEnvironment::new(environment, cwd))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::runtime_environment_without_thread;
|
||||
use codex_core::RuntimeCapabilities;
|
||||
use codex_exec_server::EnvironmentManager;
|
||||
use codex_exec_server::ExecServerRuntimePaths;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn threadless_mcp_local_fallback_rejects_isolated_runtime() {
|
||||
let environment_manager = EnvironmentManager::disabled_for_tests(test_runtime_paths());
|
||||
let error = runtime_environment_without_thread(
|
||||
&environment_manager,
|
||||
&RuntimeCapabilities::isolated(),
|
||||
test_cwd(),
|
||||
"list MCP server status without thread",
|
||||
)
|
||||
.expect_err("isolated runtime should reject local MCP fallback");
|
||||
|
||||
assert_eq!(
|
||||
error.message,
|
||||
"list MCP server status without thread requires ambient worker-local environment"
|
||||
);
|
||||
}
|
||||
|
||||
fn test_runtime_paths() -> ExecServerRuntimePaths {
|
||||
ExecServerRuntimePaths::new(
|
||||
std::env::current_exe().expect("current exe"),
|
||||
/*codex_linux_sandbox_exe*/ None,
|
||||
)
|
||||
.expect("runtime paths")
|
||||
}
|
||||
|
||||
fn test_cwd() -> std::path::PathBuf {
|
||||
AbsolutePathBuf::current_dir()
|
||||
.expect("current dir")
|
||||
.to_path_buf()
|
||||
}
|
||||
}
|
||||
|
||||
fn with_mcp_tool_call_thread_id_meta(
|
||||
meta: Option<serde_json::Value>,
|
||||
thread_id: &str,
|
||||
|
||||
@@ -657,16 +657,10 @@ impl ThreadRequestProcessor {
|
||||
.map(|response| Some(response.into()))
|
||||
}
|
||||
|
||||
async fn instruction_sources_from_config(
|
||||
runtime_capabilities: &RuntimeCapabilities,
|
||||
config: &Config,
|
||||
) -> Result<Vec<AbsolutePathBuf>, JSONRPCErrorError> {
|
||||
let file_system = runtime_capabilities
|
||||
.require_local_filesystem("load thread instruction sources")
|
||||
.map_err(|err| internal_error(err.to_string()))?;
|
||||
Ok(codex_core::AgentsMdManager::new(config)
|
||||
.instruction_sources(file_system.as_ref())
|
||||
.await)
|
||||
async fn instruction_sources_from_config(config: &Config) -> Vec<AbsolutePathBuf> {
|
||||
codex_core::AgentsMdManager::new(config)
|
||||
.instruction_sources(LOCAL_FS.as_ref())
|
||||
.await
|
||||
}
|
||||
|
||||
async fn load_thread(
|
||||
@@ -889,12 +883,14 @@ impl ThreadRequestProcessor {
|
||||
};
|
||||
let request_trace = request_context.request_trace();
|
||||
let config_manager = self.config_manager.clone();
|
||||
let runtime_capabilities = Arc::clone(&self.runtime_capabilities);
|
||||
let outgoing = Arc::clone(&listener_task_context.outgoing);
|
||||
let error_request_id = request_id.clone();
|
||||
let thread_start_task = async move {
|
||||
if let Err(error) = Self::thread_start_task(
|
||||
listener_task_context,
|
||||
config_manager,
|
||||
runtime_capabilities,
|
||||
request_id,
|
||||
app_server_client_name,
|
||||
app_server_client_version,
|
||||
@@ -979,6 +975,7 @@ impl ThreadRequestProcessor {
|
||||
async fn thread_start_task(
|
||||
listener_task_context: ListenerTaskContext,
|
||||
config_manager: ConfigManager,
|
||||
runtime_capabilities: Arc<RuntimeCapabilities>,
|
||||
request_id: ConnectionRequestId,
|
||||
app_server_client_name: Option<String>,
|
||||
app_server_client_version: Option<String>,
|
||||
@@ -1015,8 +1012,7 @@ impl ThreadRequestProcessor {
|
||||
&& config.active_project.trust_level.is_none()
|
||||
&& (requested_permissions_trust_project || effective_permissions_trust_project)
|
||||
{
|
||||
let file_system = self
|
||||
.runtime_capabilities
|
||||
let file_system = runtime_capabilities
|
||||
.require_local_filesystem("trust requested thread cwd")
|
||||
.map_err(|err| internal_error(err.to_string()))?;
|
||||
let trust_target =
|
||||
@@ -1069,8 +1065,7 @@ impl ThreadRequestProcessor {
|
||||
.map_err(|err| config_load_error(&err))?;
|
||||
}
|
||||
|
||||
let instruction_sources =
|
||||
Self::instruction_sources_from_config(&self.runtime_capabilities, &config).await?;
|
||||
let instruction_sources = Self::instruction_sources_from_config(&config).await;
|
||||
let environments = environments.unwrap_or_else(|| {
|
||||
listener_task_context
|
||||
.thread_manager
|
||||
@@ -2454,8 +2449,7 @@ impl ThreadRequestProcessor {
|
||||
}
|
||||
};
|
||||
|
||||
let instruction_sources =
|
||||
Self::instruction_sources_from_config(&self.runtime_capabilities, &config).await?;
|
||||
let instruction_sources = Self::instruction_sources_from_config(&config).await;
|
||||
let response_history = thread_history.clone();
|
||||
|
||||
match self
|
||||
@@ -2740,11 +2734,8 @@ impl ThreadRequestProcessor {
|
||||
thread_summary.session_id = existing_thread.session_configured().session_id.to_string();
|
||||
let mut config_for_instruction_sources = self.config.as_ref().clone();
|
||||
config_for_instruction_sources.cwd = config_snapshot.cwd.clone();
|
||||
let instruction_sources = Self::instruction_sources_from_config(
|
||||
&self.runtime_capabilities,
|
||||
&config_for_instruction_sources,
|
||||
)
|
||||
.await?;
|
||||
let instruction_sources =
|
||||
Self::instruction_sources_from_config(&config_for_instruction_sources).await;
|
||||
|
||||
let listener_command_tx = {
|
||||
let thread_state = thread_state.lock().await;
|
||||
@@ -3114,8 +3105,7 @@ impl ThreadRequestProcessor {
|
||||
.map_err(|err| config_load_error(&err))?;
|
||||
|
||||
let fallback_model_provider = config.model_provider_id.clone();
|
||||
let instruction_sources =
|
||||
Self::instruction_sources_from_config(&self.runtime_capabilities, &config).await?;
|
||||
let instruction_sources = Self::instruction_sources_from_config(&config).await;
|
||||
|
||||
let NewThread {
|
||||
thread_id,
|
||||
|
||||
@@ -59,8 +59,6 @@ mod thread_processor_behavior_tests {
|
||||
use codex_config::SessionThreadConfig;
|
||||
use codex_config::StaticThreadConfigLoader;
|
||||
use codex_config::ThreadConfigSource;
|
||||
use codex_core::RuntimeCapabilities;
|
||||
use codex_core::config::ConfigBuilder;
|
||||
use codex_model_provider_info::ModelProviderInfo;
|
||||
use codex_model_provider_info::WireApi;
|
||||
use codex_protocol::ThreadId;
|
||||
@@ -90,28 +88,6 @@ mod thread_processor_behavior_tests {
|
||||
use std::sync::Arc;
|
||||
use tempfile::TempDir;
|
||||
|
||||
#[tokio::test]
|
||||
async fn instruction_sources_reject_isolated_runtime_without_local_filesystem() {
|
||||
let temp_dir = TempDir::new().expect("tempdir");
|
||||
let config = ConfigBuilder::without_managed_config_for_tests()
|
||||
.codex_home(temp_dir.path().to_path_buf())
|
||||
.build()
|
||||
.await
|
||||
.expect("build config");
|
||||
|
||||
let error = ThreadRequestProcessor::instruction_sources_from_config(
|
||||
&RuntimeCapabilities::isolated(),
|
||||
&config,
|
||||
)
|
||||
.await
|
||||
.expect_err("isolated runtime should reject instruction source load");
|
||||
|
||||
assert_eq!(
|
||||
error.message,
|
||||
"load thread instruction sources requires ambient worker-local filesystem"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn validate_dynamic_tools_rejects_unsupported_input_schema() {
|
||||
let tools = vec![ApiDynamicToolSpec {
|
||||
|
||||
Reference in New Issue
Block a user