mirror of
https://github.com/openai/codex.git
synced 2026-10-21 17:48:44 +00:00
Use step environment snapshots for subagent inheritance (#55331)
## What changed - Remove `subagent_thread_environments` and the separate configured environment bindings. Spawned agents inherit the invoking step's environment snapshot. - Retain environment selections across child eviction without retaining executor bindings. Reload V2 children against the parent's captured environments, requiring matching environment IDs, working directories, and workspace roots while preserving permission validation. GitOrigin-RevId: d820c63ea20f5834ed88071d16c60683f2545406
This commit is contained in:
@@ -651,7 +651,14 @@ impl LocalAgentControl {
|
||||
};
|
||||
|
||||
let parent_thread = state.get_thread(*parent_thread_id).await.ok()?;
|
||||
Some(parent_thread.session.inherited_environment_snapshot().await)
|
||||
Some(
|
||||
parent_thread
|
||||
.session
|
||||
.services
|
||||
.turn_environments
|
||||
.snapshot()
|
||||
.await,
|
||||
)
|
||||
}
|
||||
|
||||
async fn inherited_exec_policy_for_source(
|
||||
|
||||
@@ -4,7 +4,6 @@ use super::LocalAgentControl;
|
||||
use super::LocalAgentRuntime;
|
||||
use super::runtime::AgentTreeMembership;
|
||||
use crate::agent::AgentStatus;
|
||||
use crate::agent::registry::EvictedEnvironments;
|
||||
use crate::codex_thread::CodexThread;
|
||||
use crate::config::Config;
|
||||
use crate::thread_manager::ThreadManagerState;
|
||||
@@ -77,16 +76,7 @@ impl ThreadManagerState {
|
||||
.collect();
|
||||
let runtime = &thread.session.services.local_agent_runtime;
|
||||
let _ = runtime.mailboxes.enqueue(thread_id, /*id*/ None, mail);
|
||||
// Preserve configured bindings across eviction without retaining turn-only attachments.
|
||||
// Raw bindings keep the original pending owner configuration alive.
|
||||
let environments = EvictedEnvironments {
|
||||
selections: thread.environment_selections().await,
|
||||
bindings: thread
|
||||
.session
|
||||
.services
|
||||
.turn_environments
|
||||
.configured_snapshot_now(),
|
||||
};
|
||||
let environments = thread.environment_selections().await;
|
||||
let mut threads = manager.threads.write().await;
|
||||
if threads
|
||||
.get(&thread_id)
|
||||
|
||||
@@ -362,11 +362,7 @@ impl LocalAgentControl {
|
||||
{
|
||||
return Err(CodexErr::ThreadNotFound(thread_id));
|
||||
}
|
||||
let evicted_environments = self.runtime.registry.evicted_environments(thread_id);
|
||||
let mut environment_selections = evicted_environments
|
||||
.as_ref()
|
||||
.map(|environments| environments.selections.clone());
|
||||
let evicted_environments = evicted_environments.map(|environments| environments.bindings);
|
||||
let mut environment_selections = self.runtime.registry.evicted_environments(thread_id);
|
||||
|
||||
let stored_thread = state
|
||||
.read_stored_thread(ReadThreadParams {
|
||||
@@ -422,8 +418,7 @@ impl LocalAgentControl {
|
||||
"cannot resume multi-agent v2 child {thread_id} with the current parent settings"
|
||||
))
|
||||
})?;
|
||||
let environments = parent.session.inherited_environment_snapshot().await;
|
||||
Some((parent, environments))
|
||||
Some((parent, turn.initial_environments.clone()))
|
||||
} else {
|
||||
None
|
||||
};
|
||||
@@ -495,11 +490,6 @@ impl LocalAgentControl {
|
||||
)));
|
||||
}
|
||||
if let Some(selections) = environment_selections.as_mut() {
|
||||
let parent_selections = parent_environments.all_selections();
|
||||
let ready_parent = parent_environments.refresh_readiness();
|
||||
let ready_evicted = evicted_environments
|
||||
.as_ref()
|
||||
.map(TurnEnvironmentSnapshot::refresh_readiness);
|
||||
for selection in selections {
|
||||
let environment_id = &selection.environment_id;
|
||||
let invalid_environment = |reason: &str| {
|
||||
@@ -507,22 +497,17 @@ impl LocalAgentControl {
|
||||
"cannot resume multi-agent v2 child {thread_id}: cached environment {environment_id} {reason}"
|
||||
))
|
||||
};
|
||||
// A changed parent selection does not revoke the child's retained binding.
|
||||
// If the parent still configures this workspace, keep enforcing its current
|
||||
// roots and permissions rather than restoring superseded authority.
|
||||
let owner_environments = if parent_selections.iter().any(|parent_selection| {
|
||||
parent_selection.environment_id == selection.environment_id
|
||||
&& parent_selection.cwd == selection.cwd
|
||||
}) {
|
||||
&ready_parent
|
||||
} else {
|
||||
ready_evicted.as_ref().unwrap_or(&ready_parent)
|
||||
};
|
||||
let owner_environment = owner_environments
|
||||
// Matching the attachment also keeps startup on the captured owner executor.
|
||||
let owner_environment = parent_environments
|
||||
.turn_environments()
|
||||
.find(|environment| environment.selection.has_same_workspace(selection))
|
||||
.find(|environment| {
|
||||
let parent_selection = &environment.selection;
|
||||
parent_selection.environment_id == selection.environment_id
|
||||
&& parent_selection.cwd == selection.cwd
|
||||
&& parent_selection.workspace_roots == selection.workspace_roots
|
||||
})
|
||||
.ok_or_else(|| {
|
||||
invalid_environment("has no matching ready environment binding")
|
||||
invalid_environment("no longer matches a ready parent environment")
|
||||
})?;
|
||||
let owner_config = owner_environment.config();
|
||||
let child_config = match &selection.config {
|
||||
@@ -584,24 +569,14 @@ impl LocalAgentControl {
|
||||
}
|
||||
}
|
||||
(
|
||||
Some(
|
||||
evicted_environments
|
||||
.as_ref()
|
||||
.unwrap_or(parent_environments)
|
||||
.clone(),
|
||||
),
|
||||
Some(parent_environments.clone()),
|
||||
Some(Arc::clone(&parent.session.services.exec_policy)),
|
||||
Some(parent.client_mcp_extensions()),
|
||||
)
|
||||
} else {
|
||||
(
|
||||
match &evicted_environments {
|
||||
Some(environments) => Some(environments.clone()),
|
||||
None => {
|
||||
self.inherited_environments_for_source(&state, Some(&session_source))
|
||||
.await
|
||||
}
|
||||
},
|
||||
self.inherited_environments_for_source(&state, Some(&session_source))
|
||||
.await,
|
||||
self.inherited_exec_policy_for_source(&state, Some(&session_source), &config)
|
||||
.await,
|
||||
None,
|
||||
|
||||
@@ -1175,12 +1175,9 @@ async fn cancelled_v2_send_finishes_reload_registration() {
|
||||
.unwrap(),
|
||||
child_id,
|
||||
);
|
||||
assert!(
|
||||
control
|
||||
.runtime
|
||||
.registry
|
||||
.evicted_environments(child_id)
|
||||
.is_none()
|
||||
assert_eq!(
|
||||
control.runtime.registry.evicted_environments(child_id),
|
||||
None
|
||||
);
|
||||
// Evict and reload again with the single child slot. This also catches a leaked
|
||||
// pin or pending residency reservation left behind by the cancelled sender.
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
use crate::agent::types::AgentMetadata;
|
||||
use crate::environment_selection::TurnEnvironmentSnapshot;
|
||||
use codex_protocol::AgentPath;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::error::AgentErrorContext;
|
||||
@@ -38,16 +37,9 @@ struct ActiveAgents {
|
||||
nickname_reset_count: usize,
|
||||
}
|
||||
|
||||
/// Thread defaults and executor bindings retained while a child's runtime is unloaded.
|
||||
#[derive(Clone)]
|
||||
pub(crate) struct EvictedEnvironments {
|
||||
pub(crate) selections: Vec<TurnEnvironmentSelection>,
|
||||
pub(crate) bindings: TurnEnvironmentSnapshot,
|
||||
}
|
||||
|
||||
struct RegisteredAgent {
|
||||
path: String,
|
||||
evicted_environments: Option<EvictedEnvironments>,
|
||||
evicted_environments: Option<Vec<TurnEnvironmentSelection>>,
|
||||
}
|
||||
|
||||
impl RegisteredAgent {
|
||||
@@ -182,7 +174,7 @@ impl AgentRegistry {
|
||||
pub(crate) fn save_evicted_environments(
|
||||
&self,
|
||||
thread_id: ThreadId,
|
||||
environments: EvictedEnvironments,
|
||||
environments: Vec<TurnEnvironmentSelection>,
|
||||
) {
|
||||
let mut active_agents = self
|
||||
.active_agents
|
||||
@@ -193,7 +185,10 @@ impl AgentRegistry {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn evicted_environments(&self, thread_id: ThreadId) -> Option<EvictedEnvironments> {
|
||||
pub(crate) fn evicted_environments(
|
||||
&self,
|
||||
thread_id: ThreadId,
|
||||
) -> Option<Vec<TurnEnvironmentSelection>> {
|
||||
let active_agents = self
|
||||
.active_agents
|
||||
.lock()
|
||||
|
||||
@@ -32,7 +32,7 @@ pub struct SpawnAgentOptions {
|
||||
/// Attribute delegated usage to the turn that initiated it.
|
||||
pub turn_trigger: Option<String>,
|
||||
pub root_turn_id: Option<String>,
|
||||
/// Inheritable bindings captured by the step that requested the child.
|
||||
/// Exact environment bindings from the step that requested the child.
|
||||
pub environments: Option<TurnEnvironmentSnapshot>,
|
||||
/// Plugin choices from the turn requesting the child. Omission inherits the parent's
|
||||
/// current choices; an explicit list, including an empty one, overrides them and history.
|
||||
|
||||
Reference in New Issue
Block a user