mirror of
https://github.com/openai/codex.git
synced 2026-10-20 17:38:45 +00:00
Keep host MCP and plugin configuration local with remote environments (#53990)
## Why A remote primary environment could supply its working directory and workspace roots to host plugins and MCP processes, breaking relative MCP executable resolution and applying the wrong workspace permissions. ## What changed - Build host configuration from the local environment, falling back to the session's host working directory and runtime workspace roots when no local environment is selected. Preserve explicitly empty local roots. - Use the same host configuration for MCP startup and refresh, and refresh MCP state when runtime workspace roots change. - Keep repository skill discovery paired with the primary executor's working directory and filesystem. - Box skill warmup and turn context construction futures to reduce enclosing future sizes. ## Testing Extend the MCP integration test to cover a remote primary with populated, empty, or absent local roots; relative executable resolution before and after refresh; workspace-root revocation; and remote repository skill discovery. GitOrigin-RevId: 21a73c90507b55fd94261e7bf348695ff8466a24
This commit is contained in:
@@ -13,7 +13,6 @@ use codex_exec_server::EnvironmentConnectionState;
|
||||
use codex_exec_server::EnvironmentInfo;
|
||||
use codex_exec_server::EnvironmentManager;
|
||||
use codex_exec_server::ExecServerError;
|
||||
use codex_exec_server::ExecutorFileSystem;
|
||||
use codex_exec_server::SelectedCapabilityRootsStatus;
|
||||
use codex_protocol::capabilities::CapabilityRootLocation;
|
||||
use codex_protocol::capabilities::EnvironmentCapabilityRoots;
|
||||
@@ -1152,14 +1151,6 @@ impl TurnEnvironmentSnapshot {
|
||||
self.turn_environments().next()
|
||||
}
|
||||
|
||||
/// Returns the first selected environment's host folders, even if setup is not ready yet.
|
||||
pub(crate) fn primary_workspace_roots(&self) -> Vec<AbsolutePathBuf> {
|
||||
self.primary_workspace_root_uris()
|
||||
.iter()
|
||||
.filter_map(|root| root.to_abs_path().ok())
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// Returns the first selection's executor paths, even when setup is not ready yet.
|
||||
pub(crate) fn primary_workspace_root_uris(&self) -> &[PathUri] {
|
||||
let selection = match self.environments.first() {
|
||||
@@ -1187,20 +1178,26 @@ impl TurnEnvironmentSnapshot {
|
||||
}
|
||||
|
||||
pub(crate) fn local_environment_cwd(&self) -> Option<AbsolutePathBuf> {
|
||||
self.local_selection()
|
||||
.and_then(|selection| selection.cwd.to_abs_path().ok())
|
||||
}
|
||||
|
||||
/// The local workspace remains host-owned while its executor is still starting.
|
||||
pub(crate) fn local_selection(&self) -> Option<&TurnEnvironmentSelection> {
|
||||
self.environments
|
||||
.iter()
|
||||
.find_map(|environment| match environment {
|
||||
TurnEnvironmentState::Ready(environment)
|
||||
if !environment.environment.is_remote() =>
|
||||
{
|
||||
environment.cwd().to_abs_path().ok()
|
||||
Some(&environment.selection)
|
||||
}
|
||||
TurnEnvironmentState::Ready(_) => None,
|
||||
TurnEnvironmentState::Starting(environment)
|
||||
if environment.selection.environment_id
|
||||
== codex_exec_server::LOCAL_ENVIRONMENT_ID =>
|
||||
{
|
||||
environment.selection.cwd.to_abs_path().ok()
|
||||
Some(&environment.selection)
|
||||
}
|
||||
TurnEnvironmentState::Starting(_) | TurnEnvironmentState::Failed { .. } => None,
|
||||
})
|
||||
@@ -1277,11 +1274,6 @@ impl TurnEnvironmentSnapshot {
|
||||
.collect()
|
||||
}
|
||||
|
||||
pub(crate) fn primary_filesystem(&self) -> Option<Arc<dyn ExecutorFileSystem>> {
|
||||
self.primary()
|
||||
.map(|environment| environment.environment.get_filesystem())
|
||||
}
|
||||
|
||||
pub(crate) fn single_local_environment(&self) -> Option<&TurnEnvironment> {
|
||||
if self.starting().next().is_some() {
|
||||
return None;
|
||||
|
||||
@@ -135,6 +135,7 @@ pub mod sandboxing;
|
||||
mod session_prefix;
|
||||
mod skills;
|
||||
pub(crate) use skills::maybe_emit_implicit_skill_invocation;
|
||||
#[cfg(test)]
|
||||
pub(crate) use skills::skills_load_input_from_config;
|
||||
mod stream_events_utils;
|
||||
pub mod test_support;
|
||||
|
||||
@@ -39,6 +39,7 @@ impl Session {
|
||||
next: &SessionConfiguration,
|
||||
) -> bool {
|
||||
current.cwd() != next.cwd()
|
||||
|| current.runtime_workspace_roots != next.runtime_workspace_roots
|
||||
|| current.step_settings.approval_policy.value()
|
||||
!= next.step_settings.approval_policy.value()
|
||||
|| current.step_settings.approvals_reviewer != next.step_settings.approvals_reviewer
|
||||
@@ -79,19 +80,9 @@ impl Session {
|
||||
state.active_disabled_plugin_ids.clone(),
|
||||
)
|
||||
};
|
||||
let cwd = environments
|
||||
.primary()
|
||||
.and_then(|environment| environment.cwd().to_abs_path().ok())
|
||||
.unwrap_or_else(|| session_configuration.cwd().clone());
|
||||
let config = self.build_per_turn_config(
|
||||
&session_configuration,
|
||||
cwd,
|
||||
environments.primary_workspace_roots(),
|
||||
);
|
||||
let local_process_cwd = environments
|
||||
.local_environment_cwd()
|
||||
.unwrap_or_else(|| session_configuration.cwd().clone())
|
||||
.to_path_buf();
|
||||
let config =
|
||||
self.build_per_turn_config(&session_configuration, environments.local_selection());
|
||||
let local_process_cwd = config.cwd.to_path_buf();
|
||||
|
||||
McpDesiredState {
|
||||
config: Arc::new(config),
|
||||
@@ -111,19 +102,12 @@ impl Session {
|
||||
auth: Option<CodexAuth>,
|
||||
mcp_projection: McpRuntimeProjection,
|
||||
resolved_environments: &TurnEnvironmentSnapshot,
|
||||
mcp_runtime_cwd: PathBuf,
|
||||
) -> anyhow::Result<()> {
|
||||
let cwd = AbsolutePathBuf::from_absolute_path(mcp_runtime_cwd)
|
||||
.unwrap_or_else(|_| session_configuration.cwd().clone());
|
||||
let config = self.build_per_turn_config(
|
||||
session_configuration,
|
||||
cwd,
|
||||
resolved_environments.primary_workspace_roots(),
|
||||
resolved_environments.local_selection(),
|
||||
);
|
||||
let local_process_cwd = resolved_environments
|
||||
.local_environment_cwd()
|
||||
.unwrap_or_else(|| session_configuration.cwd().clone())
|
||||
.to_path_buf();
|
||||
let local_process_cwd = config.cwd.to_path_buf();
|
||||
let desired = McpDesiredState {
|
||||
config: Arc::new(config),
|
||||
auth,
|
||||
|
||||
@@ -53,7 +53,6 @@ use crate::session::step_settings::ResolvedStepSettings;
|
||||
use crate::session::step_settings::StepSettings;
|
||||
use crate::session::turn_context::TurnEnvironment;
|
||||
use crate::shell_snapshot::SnapshotCredentialBrokerState;
|
||||
use crate::skills_load_input_from_config;
|
||||
use crate::stream_events_utils::mark_thread_memory_mode_polluted_if_external_context;
|
||||
use crate::turn_metadata::TurnMetadataState;
|
||||
use crate::turn_timing::now_unix_timestamp_ms;
|
||||
|
||||
@@ -646,11 +646,13 @@ async fn warm_plugins_and_skills_for_session_init(
|
||||
return Vec::new();
|
||||
}
|
||||
|
||||
let fs = turn_environments.primary_filesystem();
|
||||
let effective_skill_roots = plugin_outcome.effective_plugin_skill_roots();
|
||||
let plugin_skill_snapshots = plugins_manager.plugin_skill_snapshots_for_config(&plugins_input);
|
||||
let skills_input = skills_load_input_from_config(config.as_ref(), effective_skill_roots)
|
||||
.with_plugin_skill_snapshots(plugin_skill_snapshots);
|
||||
let (skills_input, fs) = crate::skills::skills_load_input_for_environments(
|
||||
&config,
|
||||
&plugins_manager,
|
||||
effective_skill_roots,
|
||||
turn_environments,
|
||||
);
|
||||
skills_service
|
||||
.snapshot_for_config(&skills_input, fs)
|
||||
.await
|
||||
@@ -1173,11 +1175,6 @@ impl Session {
|
||||
let mcp_originator = session_configuration.originator.clone();
|
||||
let mcp_session_source = session_configuration.session_source.clone();
|
||||
let mcp_disabled_plugin_ids = session_configuration.disabled_plugin_ids.clone();
|
||||
let mcp_runtime_cwd = environment_selections
|
||||
.first()
|
||||
.and_then(|environment| environment.cwd.to_abs_path().ok())
|
||||
.map(|cwd| cwd.to_path_buf())
|
||||
.unwrap_or_else(|| session_configuration.cwd().to_path_buf());
|
||||
let auth_and_mcp_fut = async move {
|
||||
let auth = mcp_auth.await;
|
||||
if config_for_mcp.features.plugin_recommendations_enabled() {
|
||||
@@ -1480,14 +1477,15 @@ impl Session {
|
||||
session_configuration.environments = turn_environments.selections();
|
||||
let resolved_environments = turn_environments.snapshot().await;
|
||||
let agents_md_manager = Arc::new(AgentsMdManager::new(instructions));
|
||||
let plugin_skill_warmup = warm_plugins_and_skills_for_session_init(
|
||||
// Keep discovery's future out of the large embedded startup/resume frame.
|
||||
let plugin_skill_warmup = Box::pin(warm_plugins_and_skills_for_session_init(
|
||||
Arc::clone(&config),
|
||||
Arc::clone(&plugins_manager),
|
||||
Arc::clone(&skills_service),
|
||||
&resolved_environments,
|
||||
extensions.as_ref(),
|
||||
&session_configuration.session_source,
|
||||
)
|
||||
))
|
||||
.instrument(info_span!(
|
||||
"session_init.plugin_skill_warmup",
|
||||
otel.name = "session_init.plugin_skill_warmup",
|
||||
@@ -1943,7 +1941,6 @@ impl Session {
|
||||
latest_auth,
|
||||
mcp_projection,
|
||||
&resolved_environments,
|
||||
mcp_runtime_cwd,
|
||||
)
|
||||
.await?;
|
||||
sess.start_mcp_prewarm_worker(mcp_prewarm_rx, mcp_auth_changes);
|
||||
|
||||
@@ -7022,10 +7022,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
|
||||
};
|
||||
let per_turn_config = session.build_per_turn_config(
|
||||
&session_configuration,
|
||||
session_configuration.cwd().clone(),
|
||||
crate::environment_selection::ThreadEnvironments::primary_workspace_roots_for(
|
||||
&session_configuration.environments,
|
||||
),
|
||||
resolved_turn_environments.local_selection(),
|
||||
);
|
||||
let plugins_input = per_turn_config.plugins_config_input();
|
||||
let plugin_outcome = session
|
||||
@@ -9290,10 +9287,7 @@ where
|
||||
});
|
||||
let per_turn_config = session.build_per_turn_config(
|
||||
&session_configuration,
|
||||
session_configuration.cwd().clone(),
|
||||
crate::environment_selection::ThreadEnvironments::primary_workspace_roots_for(
|
||||
&session_configuration.environments,
|
||||
),
|
||||
resolved_turn_environments.local_selection(),
|
||||
);
|
||||
let plugins_input = per_turn_config.plugins_config_input();
|
||||
let plugin_outcome = session
|
||||
|
||||
@@ -939,13 +939,26 @@ impl Session {
|
||||
pub(crate) fn build_per_turn_config(
|
||||
&self,
|
||||
session_configuration: &SessionConfiguration,
|
||||
cwd: AbsolutePathBuf,
|
||||
workspace_roots: Vec<AbsolutePathBuf>,
|
||||
local_environment: Option<&TurnEnvironmentSelection>,
|
||||
) -> Config {
|
||||
// todo(aibrahim): store this state somewhere else so we don't need to mut config
|
||||
let config = session_configuration.original_config_do_not_use.clone();
|
||||
let mut per_turn_config = (*config).clone();
|
||||
per_turn_config.cwd = cwd;
|
||||
// Config is consumed by host plugins and MCP processes. Executor paths and
|
||||
// permissions remain on their captured environment attachments.
|
||||
per_turn_config.cwd = local_environment
|
||||
.and_then(|environment| environment.cwd.to_abs_path().ok())
|
||||
.unwrap_or_else(|| session_configuration.cwd().clone());
|
||||
let workspace_roots = local_environment.map_or_else(
|
||||
|| session_configuration.runtime_workspace_roots.clone(),
|
||||
|environment| {
|
||||
environment
|
||||
.workspace_roots
|
||||
.iter()
|
||||
.filter_map(|root| root.to_abs_path().ok())
|
||||
.collect()
|
||||
},
|
||||
);
|
||||
per_turn_config.permissions.approval_policy =
|
||||
session_configuration.step_settings.approval_policy.clone();
|
||||
per_turn_config.workspace_roots = workspace_roots.clone();
|
||||
@@ -989,14 +1002,15 @@ impl Session {
|
||||
&self,
|
||||
session_configuration: &SessionConfiguration,
|
||||
) -> Config {
|
||||
let workspace_roots =
|
||||
crate::environment_selection::ThreadEnvironments::primary_workspace_roots_for(
|
||||
&session_configuration.environments,
|
||||
);
|
||||
let mut config = self.build_per_turn_config(
|
||||
session_configuration,
|
||||
session_configuration.cwd().clone(),
|
||||
workspace_roots,
|
||||
session_configuration
|
||||
.environments
|
||||
.iter()
|
||||
.find(|environment| {
|
||||
environment.environment_id == codex_exec_server::LOCAL_ENVIRONMENT_ID
|
||||
&& !matches!(environment.config, EnvironmentConfigState::Failed(_))
|
||||
}),
|
||||
);
|
||||
config.model = Some(
|
||||
session_configuration
|
||||
@@ -1210,14 +1224,15 @@ impl Session {
|
||||
turn_environments: TurnEnvironmentSnapshot,
|
||||
options: NewTurnContextOptions,
|
||||
) -> Arc<TurnContext> {
|
||||
self.new_turn_context_from_configuration(
|
||||
// Keep context construction out of the enclosing turn and history futures.
|
||||
Box::pin(self.new_turn_context_from_configuration(
|
||||
sub_id,
|
||||
session_configuration,
|
||||
turn_environments,
|
||||
options,
|
||||
TurnContextBuildMode::Full,
|
||||
self.git_enrichment_policy,
|
||||
)
|
||||
))
|
||||
.await
|
||||
}
|
||||
|
||||
@@ -1227,14 +1242,14 @@ impl Session {
|
||||
session_configuration: SessionConfiguration,
|
||||
) -> Arc<TurnContext> {
|
||||
let turn_environments = self.services.turn_environments.snapshot().await;
|
||||
self.new_turn_context_from_configuration(
|
||||
Box::pin(self.new_turn_context_from_configuration(
|
||||
sub_id,
|
||||
session_configuration,
|
||||
turn_environments,
|
||||
NewTurnContextOptions::default(),
|
||||
TurnContextBuildMode::StartupPrewarm,
|
||||
GitEnrichmentPolicy::Skip,
|
||||
)
|
||||
))
|
||||
.await
|
||||
}
|
||||
|
||||
@@ -1249,17 +1264,12 @@ impl Session {
|
||||
git_enrichment_policy: GitEnrichmentPolicy,
|
||||
) -> Arc<TurnContext> {
|
||||
let primary_turn_environment = turn_environments.primary();
|
||||
// TODO(anp): Migrate per-turn config and legacy TurnContext cwd consumers to PathUri so
|
||||
// a foreign primary environment does not fall back to the session's host cwd.
|
||||
let per_turn_config =
|
||||
self.build_per_turn_config(&session_configuration, turn_environments.local_selection());
|
||||
// Legacy turn consumers still refer to the primary executor's repository.
|
||||
let cwd = primary_turn_environment
|
||||
.as_ref()
|
||||
.and_then(|turn_environment| turn_environment.cwd().to_abs_path().ok())
|
||||
.and_then(|environment| environment.cwd().to_abs_path().ok())
|
||||
.unwrap_or_else(|| session_configuration.cwd().clone());
|
||||
let per_turn_config = self.build_per_turn_config(
|
||||
&session_configuration,
|
||||
cwd.clone(),
|
||||
turn_environments.primary_workspace_roots(),
|
||||
);
|
||||
let network_permission_profile = primary_turn_environment
|
||||
.map(TurnEnvironment::permission_profile)
|
||||
.cloned()
|
||||
@@ -1308,15 +1318,12 @@ impl Session {
|
||||
HostSkillsSnapshot::new(Arc::new(SkillLoadOutcome::default()))
|
||||
} else {
|
||||
let effective_skill_roots = plugin_outcome.effective_plugin_skill_roots();
|
||||
let plugin_skill_snapshots = self
|
||||
.services
|
||||
.plugins_manager
|
||||
.plugin_skill_snapshots_for_config(&plugins_input);
|
||||
let skills_input =
|
||||
skills_load_input_from_config(&per_turn_config, effective_skill_roots)
|
||||
.with_plugin_skill_snapshots(plugin_skill_snapshots);
|
||||
let fs = primary_turn_environment
|
||||
.map(|turn_environment| turn_environment.environment.get_filesystem());
|
||||
let (skills_input, fs) = crate::skills::skills_load_input_for_environments(
|
||||
&per_turn_config,
|
||||
&self.services.plugins_manager,
|
||||
effective_skill_roots,
|
||||
&turn_environments,
|
||||
);
|
||||
self.services
|
||||
.skills_service
|
||||
.snapshot_for_config(&skills_input, fs)
|
||||
@@ -1444,14 +1451,14 @@ impl Session {
|
||||
async fn new_default_turn_for(&self, build_mode: TurnContextBuildMode) -> Arc<TurnContext> {
|
||||
let session_configuration = self.default_turn_configuration().await;
|
||||
let turn_environments = self.services.turn_environments.snapshot().await;
|
||||
self.new_turn_context_from_configuration(
|
||||
Box::pin(self.new_turn_context_from_configuration(
|
||||
self.next_internal_sub_id(),
|
||||
session_configuration,
|
||||
turn_environments,
|
||||
NewTurnContextOptions::default(),
|
||||
build_mode,
|
||||
self.git_enrichment_policy,
|
||||
)
|
||||
))
|
||||
.await
|
||||
}
|
||||
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
use crate::config::Config;
|
||||
use crate::environment_selection::TurnEnvironmentSnapshot;
|
||||
use crate::session::session::Session;
|
||||
use crate::session::turn_context::TurnContext;
|
||||
use codex_analytics::InvocationType;
|
||||
@@ -6,6 +7,8 @@ use codex_analytics::SkillInvocation;
|
||||
use codex_analytics::SkillInvocationLocation;
|
||||
use codex_analytics::TrackEventsContext;
|
||||
use codex_analytics::build_track_events_context;
|
||||
use codex_core_plugins::PluginsManager;
|
||||
use codex_exec_server::ExecutorFileSystem;
|
||||
use codex_extension_api::SkillInvocationInput;
|
||||
use codex_extension_api::SkillInvocationKind;
|
||||
use codex_otel::SkillInvocationEvent;
|
||||
@@ -21,11 +24,13 @@ use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use codex_utils_plugins::PluginSkillRoot;
|
||||
use std::collections::HashSet;
|
||||
use std::sync::Arc;
|
||||
use tokio::sync::Mutex;
|
||||
|
||||
#[derive(Debug, Default)]
|
||||
struct ImplicitSkillInvocations(Mutex<HashSet<String>>);
|
||||
|
||||
#[cfg(test)]
|
||||
pub(crate) fn skills_load_input_from_config(
|
||||
config: &Config,
|
||||
effective_skill_roots: Vec<PluginSkillRoot>,
|
||||
@@ -37,6 +42,37 @@ pub(crate) fn skills_load_input_from_config(
|
||||
)
|
||||
}
|
||||
|
||||
/// Pair repository discovery inputs with their captured filesystem owner.
|
||||
/// Keep preparation synchronous so it adds no nested startup or resume future.
|
||||
pub(crate) fn skills_load_input_for_environments(
|
||||
config: &Config,
|
||||
plugins_manager: &PluginsManager,
|
||||
effective_skill_roots: Vec<PluginSkillRoot>,
|
||||
environments: &TurnEnvironmentSnapshot,
|
||||
) -> (HostSkillsLoadInput, Option<Arc<dyn ExecutorFileSystem>>) {
|
||||
// Preserve primary-repository discovery independently of the host config cwd.
|
||||
let workspace = environments.primary().and_then(|environment| {
|
||||
environment
|
||||
.cwd()
|
||||
.to_abs_path()
|
||||
.ok()
|
||||
.map(|cwd| (cwd, environment.environment.get_filesystem()))
|
||||
});
|
||||
let (cwd, fs) = match workspace {
|
||||
Some((cwd, fs)) => (cwd, Some(fs)),
|
||||
None => (config.cwd.clone(), None),
|
||||
};
|
||||
let input = HostSkillsLoadInput::new(
|
||||
cwd,
|
||||
effective_skill_roots,
|
||||
config.config_layer_stack.clone(),
|
||||
)
|
||||
.with_plugin_skill_snapshots(
|
||||
plugins_manager.plugin_skill_snapshots_for_config(&config.plugins_config_input()),
|
||||
);
|
||||
(input, fs)
|
||||
}
|
||||
|
||||
pub(crate) async fn emit_explicit_skill_invocations(
|
||||
sess: &Session,
|
||||
turn_context: &TurnContext,
|
||||
|
||||
Reference in New Issue
Block a user