mirror of
https://github.com/openai/codex.git
synced 2026-05-16 09:12:54 +00:00
Fix environment startup defaults and sample build
Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
@@ -85,7 +85,6 @@ pub(crate) fn resolve_environment_selections(
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use codex_exec_server::ExecServerRuntimePaths;
|
||||
use codex_exec_server::LOCAL_ENVIRONMENT_ID;
|
||||
use codex_exec_server::REMOTE_ENVIRONMENT_ID;
|
||||
use codex_protocol::protocol::TurnEnvironmentSelection;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
@@ -111,16 +110,10 @@ mod tests {
|
||||
|
||||
assert_eq!(
|
||||
default_thread_environment_selections(&manager, &cwd),
|
||||
vec![
|
||||
TurnEnvironmentSelection {
|
||||
environment_id: REMOTE_ENVIRONMENT_ID.to_string(),
|
||||
cwd: cwd.clone(),
|
||||
},
|
||||
TurnEnvironmentSelection {
|
||||
environment_id: LOCAL_ENVIRONMENT_ID.to_string(),
|
||||
cwd,
|
||||
},
|
||||
]
|
||||
vec![TurnEnvironmentSelection {
|
||||
environment_id: REMOTE_ENVIRONMENT_ID.to_string(),
|
||||
cwd,
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -439,8 +439,26 @@ args = ["dev", "cd /tmp && true"]
|
||||
})
|
||||
.expect("environment context prompt item");
|
||||
assert!(environment_context.contains("<environments>"));
|
||||
assert!(environment_context.contains("<environment id=\"local\">"));
|
||||
assert!(environment_context.contains("<environment id=\"dev\">"));
|
||||
let cwd = thread.session_configured.cwd.display().to_string();
|
||||
let dev_entry = format!(
|
||||
r#"<environment id="dev">
|
||||
<cwd>{cwd}</cwd>
|
||||
<shell>"#
|
||||
);
|
||||
let local_entry = format!(
|
||||
r#"<environment id="local">
|
||||
<cwd>{cwd}</cwd>
|
||||
<shell>"#
|
||||
);
|
||||
let dev_position = environment_context
|
||||
.find(&dev_entry)
|
||||
.expect("dev environment entry");
|
||||
let local_position = environment_context
|
||||
.find(&local_entry)
|
||||
.expect("local environment entry");
|
||||
assert!(dev_position < local_position);
|
||||
assert!(!environment_context.contains("\n <cwd>"));
|
||||
assert!(!environment_context.contains("\n <shell>"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
@@ -41,7 +41,7 @@ pub const CODEX_EXEC_SERVER_URL_ENV_VAR: &str = "CODEX_EXEC_SERVER_URL";
|
||||
#[derive(Debug)]
|
||||
pub struct EnvironmentManager {
|
||||
default_environment: Option<String>,
|
||||
configured_environment_ids: Vec<String>,
|
||||
startup_environment_ids: Vec<String>,
|
||||
environments: HashMap<String, Arc<Environment>>,
|
||||
local_environment: Arc<Environment>,
|
||||
}
|
||||
@@ -54,7 +54,7 @@ impl EnvironmentManager {
|
||||
pub fn default_for_tests() -> Self {
|
||||
Self {
|
||||
default_environment: Some(LOCAL_ENVIRONMENT_ID.to_string()),
|
||||
configured_environment_ids: vec![LOCAL_ENVIRONMENT_ID.to_string()],
|
||||
startup_environment_ids: vec![LOCAL_ENVIRONMENT_ID.to_string()],
|
||||
environments: HashMap::from([(
|
||||
LOCAL_ENVIRONMENT_ID.to_string(),
|
||||
Arc::new(Environment::default_for_tests()),
|
||||
@@ -67,7 +67,7 @@ impl EnvironmentManager {
|
||||
pub fn disabled_for_tests(local_runtime_paths: ExecServerRuntimePaths) -> Self {
|
||||
Self {
|
||||
default_environment: None,
|
||||
configured_environment_ids: Vec::new(),
|
||||
startup_environment_ids: Vec::new(),
|
||||
environments: HashMap::new(),
|
||||
local_environment: Arc::new(Environment::local(local_runtime_paths)),
|
||||
}
|
||||
@@ -137,6 +137,7 @@ impl EnvironmentManager {
|
||||
let EnvironmentProviderSnapshot {
|
||||
environments,
|
||||
default,
|
||||
include_all_environments_by_default,
|
||||
} = snapshot;
|
||||
let mut configured_environment_ids = Vec::with_capacity(environments.len());
|
||||
let mut environment_map = HashMap::with_capacity(environments.len());
|
||||
@@ -168,10 +169,25 @@ impl EnvironmentManager {
|
||||
}
|
||||
};
|
||||
let local_environment = Arc::new(Environment::local(local_runtime_paths));
|
||||
let startup_environment_ids = match default_environment.as_ref() {
|
||||
None => Vec::new(),
|
||||
Some(default_environment_id) if include_all_environments_by_default => {
|
||||
let mut environment_ids = Vec::with_capacity(configured_environment_ids.len());
|
||||
environment_ids.push(default_environment_id.clone());
|
||||
environment_ids.extend(
|
||||
configured_environment_ids
|
||||
.iter()
|
||||
.filter(|environment_id| environment_id.as_str() != default_environment_id)
|
||||
.cloned(),
|
||||
);
|
||||
environment_ids
|
||||
}
|
||||
Some(default_environment_id) => vec![default_environment_id.clone()],
|
||||
};
|
||||
|
||||
Ok(Self {
|
||||
default_environment,
|
||||
configured_environment_ids,
|
||||
startup_environment_ids,
|
||||
environments: environment_map,
|
||||
local_environment,
|
||||
})
|
||||
@@ -191,22 +207,7 @@ impl EnvironmentManager {
|
||||
|
||||
/// Returns the ordered environment ids used for new thread startup.
|
||||
pub fn default_environment_ids(&self) -> Vec<String> {
|
||||
let Some(default_environment_id) = self.default_environment.as_deref() else {
|
||||
return Vec::new();
|
||||
};
|
||||
if self.configured_environment_ids.len() <= 1 {
|
||||
return vec![default_environment_id.to_string()];
|
||||
}
|
||||
|
||||
let mut environment_ids = Vec::with_capacity(self.configured_environment_ids.len());
|
||||
environment_ids.push(default_environment_id.to_string());
|
||||
environment_ids.extend(
|
||||
self.configured_environment_ids
|
||||
.iter()
|
||||
.filter(|environment_id| environment_id.as_str() != default_environment_id)
|
||||
.cloned(),
|
||||
);
|
||||
environment_ids
|
||||
self.startup_environment_ids.clone()
|
||||
}
|
||||
|
||||
/// Returns the local environment instance used for internal runtime work.
|
||||
@@ -488,6 +489,7 @@ mod tests {
|
||||
.expect("remote environment"),
|
||||
)],
|
||||
default: EnvironmentDefault::EnvironmentId(REMOTE_ENVIRONMENT_ID.to_string()),
|
||||
include_all_environments_by_default: false,
|
||||
},
|
||||
};
|
||||
let manager = EnvironmentManager::from_provider(&provider, test_runtime_paths())
|
||||
@@ -514,6 +516,7 @@ mod tests {
|
||||
snapshot: EnvironmentProviderSnapshot {
|
||||
environments: vec![("".to_string(), Environment::default_for_tests())],
|
||||
default: EnvironmentDefault::Disabled,
|
||||
include_all_environments_by_default: false,
|
||||
},
|
||||
};
|
||||
let err = EnvironmentManager::from_provider(&provider, test_runtime_paths())
|
||||
@@ -542,6 +545,7 @@ mod tests {
|
||||
),
|
||||
],
|
||||
default: EnvironmentDefault::EnvironmentId("devbox".to_string()),
|
||||
include_all_environments_by_default: true,
|
||||
},
|
||||
};
|
||||
let manager = EnvironmentManager::from_provider(&provider, test_runtime_paths())
|
||||
@@ -565,6 +569,7 @@ mod tests {
|
||||
Environment::default_for_tests(),
|
||||
)],
|
||||
default: EnvironmentDefault::Disabled,
|
||||
include_all_environments_by_default: true,
|
||||
},
|
||||
};
|
||||
let manager = EnvironmentManager::from_provider(&provider, test_runtime_paths())
|
||||
@@ -585,6 +590,7 @@ mod tests {
|
||||
Environment::default_for_tests(),
|
||||
)],
|
||||
default: EnvironmentDefault::EnvironmentId("missing".to_string()),
|
||||
include_all_environments_by_default: true,
|
||||
},
|
||||
};
|
||||
let err = EnvironmentManager::from_provider(&provider, test_runtime_paths())
|
||||
|
||||
@@ -26,6 +26,7 @@ pub trait EnvironmentProvider: Send + Sync {
|
||||
pub struct EnvironmentProviderSnapshot {
|
||||
pub environments: Vec<(String, Environment)>,
|
||||
pub default: EnvironmentDefault,
|
||||
pub include_all_environments_by_default: bool,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, PartialEq, Eq)]
|
||||
@@ -82,6 +83,7 @@ impl DefaultEnvironmentProvider {
|
||||
EnvironmentProviderSnapshot {
|
||||
environments,
|
||||
default,
|
||||
include_all_environments_by_default: false,
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -132,6 +134,7 @@ mod tests {
|
||||
let EnvironmentProviderSnapshot {
|
||||
environments,
|
||||
default,
|
||||
include_all_environments_by_default,
|
||||
} = snapshot;
|
||||
let environments: HashMap<_, _> = environments.into_iter().collect();
|
||||
|
||||
@@ -145,6 +148,7 @@ mod tests {
|
||||
default,
|
||||
EnvironmentDefault::EnvironmentId(LOCAL_ENVIRONMENT_ID.to_string())
|
||||
);
|
||||
assert!(!include_all_environments_by_default);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -158,6 +162,7 @@ mod tests {
|
||||
let EnvironmentProviderSnapshot {
|
||||
environments,
|
||||
default,
|
||||
include_all_environments_by_default,
|
||||
} = snapshot;
|
||||
let environments: HashMap<_, _> = environments.into_iter().collect();
|
||||
|
||||
@@ -167,6 +172,7 @@ mod tests {
|
||||
default,
|
||||
EnvironmentDefault::EnvironmentId(LOCAL_ENVIRONMENT_ID.to_string())
|
||||
);
|
||||
assert!(!include_all_environments_by_default);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -180,12 +186,14 @@ mod tests {
|
||||
let EnvironmentProviderSnapshot {
|
||||
environments,
|
||||
default,
|
||||
include_all_environments_by_default,
|
||||
} = snapshot;
|
||||
let environments: HashMap<_, _> = environments.into_iter().collect();
|
||||
|
||||
assert!(!environments[LOCAL_ENVIRONMENT_ID].is_remote());
|
||||
assert!(!environments.contains_key(REMOTE_ENVIRONMENT_ID));
|
||||
assert_eq!(default, EnvironmentDefault::Disabled);
|
||||
assert!(!include_all_environments_by_default);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -199,6 +207,7 @@ mod tests {
|
||||
let EnvironmentProviderSnapshot {
|
||||
environments,
|
||||
default,
|
||||
include_all_environments_by_default,
|
||||
} = snapshot;
|
||||
let environments: HashMap<_, _> = environments.into_iter().collect();
|
||||
|
||||
@@ -213,6 +222,7 @@ mod tests {
|
||||
default,
|
||||
EnvironmentDefault::EnvironmentId(REMOTE_ENVIRONMENT_ID.to_string())
|
||||
);
|
||||
assert!(!include_all_environments_by_default);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
@@ -100,6 +100,7 @@ impl EnvironmentProvider for TomlEnvironmentProvider {
|
||||
Ok(EnvironmentProviderSnapshot {
|
||||
environments,
|
||||
default: self.default.clone(),
|
||||
include_all_environments_by_default: true,
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -302,15 +303,6 @@ mod tests {
|
||||
|
||||
#[tokio::test]
|
||||
async fn toml_provider_adds_implicit_local_and_configured_environments() {
|
||||
let ssh_transport = ExecServerTransportParams::StdioCommand(StdioExecServerCommand {
|
||||
program: "ssh".to_string(),
|
||||
args: vec![
|
||||
"dev".to_string(),
|
||||
"codex exec-server --listen stdio".to_string(),
|
||||
],
|
||||
env: HashMap::from([("CODEX_LOG".to_string(), "debug".to_string())]),
|
||||
cwd: None,
|
||||
});
|
||||
let provider = TomlEnvironmentProvider::new(EnvironmentsToml {
|
||||
default: Some("ssh-dev".to_string()),
|
||||
environments: vec![
|
||||
@@ -344,7 +336,16 @@ mod tests {
|
||||
let EnvironmentProviderSnapshot {
|
||||
environments,
|
||||
default,
|
||||
include_all_environments_by_default,
|
||||
} = snapshot;
|
||||
let environment_ids: Vec<_> = environments
|
||||
.iter()
|
||||
.map(|(id, _environment)| id.as_str())
|
||||
.collect();
|
||||
assert_eq!(
|
||||
environment_ids,
|
||||
vec![LOCAL_ENVIRONMENT_ID, "devbox", "ssh-dev"]
|
||||
);
|
||||
let environments: HashMap<_, _> = environments.into_iter().collect();
|
||||
|
||||
assert!(!environments[LOCAL_ENVIRONMENT_ID].is_remote());
|
||||
@@ -352,14 +353,13 @@ mod tests {
|
||||
environments["devbox"].exec_server_url(),
|
||||
Some("ws://127.0.0.1:8765")
|
||||
);
|
||||
assert_eq!(provider.environments[1].0, "ssh-dev");
|
||||
assert_eq!(provider.environments[1].1, ssh_transport);
|
||||
assert!(environments["ssh-dev"].is_remote());
|
||||
assert_eq!(environments["ssh-dev"].exec_server_url(), None);
|
||||
assert_eq!(
|
||||
default,
|
||||
EnvironmentDefault::EnvironmentId("ssh-dev".to_string())
|
||||
);
|
||||
assert!(include_all_environments_by_default);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
@@ -57,6 +57,7 @@ use codex_core_api::built_in_model_providers;
|
||||
use codex_core_api::find_codex_home;
|
||||
use codex_core_api::init_state_db_from_config;
|
||||
use codex_core_api::item_event_to_server_notification;
|
||||
use codex_core_api::resolve_installation_id;
|
||||
use codex_core_api::set_default_originator;
|
||||
use codex_core_api::thread_store_from_config;
|
||||
|
||||
@@ -119,6 +120,7 @@ async fn run_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> {
|
||||
let environment_manager = Arc::new(
|
||||
EnvironmentManager::from_codex_home(config.codex_home.clone(), local_runtime_paths).await?,
|
||||
);
|
||||
let installation_id = resolve_installation_id(&config.codex_home).await?;
|
||||
let thread_manager = ThreadManager::new(
|
||||
&config,
|
||||
auth_manager,
|
||||
@@ -128,6 +130,7 @@ async fn run_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> {
|
||||
state_db,
|
||||
Arc::clone(&thread_store),
|
||||
agent_graph_store,
|
||||
installation_id,
|
||||
);
|
||||
|
||||
let NewThread {
|
||||
|
||||
Reference in New Issue
Block a user