mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] Add environment shell info (#26480)
## Why
Shell detection needs to be available through the `Environment`
abstraction so callers can ask the selected local or remote environment
for shell metadata without adding a separate HTTP endpoint or parallel
info-source path. This keeps shell metadata shaped like the existing
environment-owned filesystem capability and lets remote environments
answer through exec-server JSON-RPC.
## What changed
- Added `environment/info` to the exec-server protocol/client/server and
exposed `Environment::info()`.
- Added local and remote environment info providers on `Environment`,
following the existing capability-provider pattern used for filesystem
access.
- Moved the shared shell detection logic into `codex-shell-command` and
kept core shell APIs as wrappers around that implementation.
- Returned shell metadata as `EnvironmentInfo { shell: ShellInfo }`
using the existing shell detection path.
- Added a remote environment test that calls `Environment::info()`
through an exec-server-backed environment.
## Validation
- `git diff --check`
- `just test -p codex-shell-command`
- `just test -p codex-core -E 'test(/shell::tests::/)'`\n- `just test -p
codex-exec-server environment`
This commit is contained in:
committed by
GitHub
Unverified
parent
64e0829cab
commit
6a6a5f925e
@@ -29,6 +29,7 @@ use crate::connection::JsonRpcConnection;
|
||||
use crate::process::ExecProcessEvent;
|
||||
use crate::process::ExecProcessEventLog;
|
||||
use crate::process::ExecProcessEventReceiver;
|
||||
use crate::protocol::ENVIRONMENT_INFO_METHOD;
|
||||
use crate::protocol::EXEC_CLOSED_METHOD;
|
||||
use crate::protocol::EXEC_EXITED_METHOD;
|
||||
use crate::protocol::EXEC_METHOD;
|
||||
@@ -36,6 +37,7 @@ use crate::protocol::EXEC_OUTPUT_DELTA_METHOD;
|
||||
use crate::protocol::EXEC_READ_METHOD;
|
||||
use crate::protocol::EXEC_TERMINATE_METHOD;
|
||||
use crate::protocol::EXEC_WRITE_METHOD;
|
||||
use crate::protocol::EnvironmentInfo;
|
||||
use crate::protocol::ExecClosedNotification;
|
||||
use crate::protocol::ExecExitedNotification;
|
||||
use crate::protocol::ExecOutputDeltaNotification;
|
||||
@@ -279,6 +281,12 @@ impl HttpClient for LazyRemoteExecServerClient {
|
||||
}
|
||||
}
|
||||
|
||||
impl LazyRemoteExecServerClient {
|
||||
pub(crate) async fn environment_info(&self) -> Result<EnvironmentInfo, ExecServerError> {
|
||||
self.get().await?.environment_info().await
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, thiserror::Error)]
|
||||
pub enum ExecServerError {
|
||||
#[error("failed to spawn exec-server: {0}")]
|
||||
@@ -363,6 +371,10 @@ impl ExecServerClient {
|
||||
self.call(EXEC_METHOD, ¶ms).await
|
||||
}
|
||||
|
||||
pub async fn environment_info(&self) -> Result<EnvironmentInfo, ExecServerError> {
|
||||
self.call(ENVIRONMENT_INFO_METHOD, &()).await
|
||||
}
|
||||
|
||||
pub async fn read(&self, params: ReadParams) -> Result<ReadResponse, ExecServerError> {
|
||||
self.call(EXEC_READ_METHOD, ¶ms).await
|
||||
}
|
||||
|
||||
@@ -2,6 +2,9 @@ use std::collections::HashMap;
|
||||
use std::sync::Arc;
|
||||
use std::sync::RwLock;
|
||||
|
||||
use futures::FutureExt;
|
||||
use futures::future::BoxFuture;
|
||||
|
||||
use crate::ExecServerError;
|
||||
use crate::ExecServerRuntimePaths;
|
||||
use crate::ExecutorFileSystem;
|
||||
@@ -18,8 +21,11 @@ use crate::environment_toml::environment_provider_from_codex_home;
|
||||
use crate::local_file_system::LocalFileSystem;
|
||||
use crate::local_process::LocalProcess;
|
||||
use crate::process::ExecBackend;
|
||||
use crate::protocol::EnvironmentInfo;
|
||||
use crate::protocol::ShellInfo;
|
||||
use crate::remote_file_system::RemoteFileSystem;
|
||||
use crate::remote_process::RemoteProcess;
|
||||
use codex_shell_command::shell_detect::DetectedShell;
|
||||
|
||||
pub const CODEX_EXEC_SERVER_URL_ENV_VAR: &str = "CODEX_EXEC_SERVER_URL";
|
||||
|
||||
@@ -286,18 +292,49 @@ impl EnvironmentManager {
|
||||
pub struct Environment {
|
||||
exec_server_url: Option<String>,
|
||||
remote_transport: Option<ExecServerTransportParams>,
|
||||
info_provider: Arc<dyn EnvironmentInfoProvider>,
|
||||
exec_backend: Arc<dyn ExecBackend>,
|
||||
filesystem: Arc<dyn ExecutorFileSystem>,
|
||||
http_client: Arc<dyn HttpClient>,
|
||||
local_runtime_paths: Option<ExecServerRuntimePaths>,
|
||||
}
|
||||
|
||||
/// Provides environment metadata from either a local environment or a remote exec-server.
|
||||
trait EnvironmentInfoProvider: Send + Sync {
|
||||
fn info(&self) -> BoxFuture<'_, Result<EnvironmentInfo, ExecServerError>>;
|
||||
}
|
||||
|
||||
struct LocalEnvironmentInfoProvider;
|
||||
|
||||
impl EnvironmentInfoProvider for LocalEnvironmentInfoProvider {
|
||||
fn info(&self) -> BoxFuture<'_, Result<EnvironmentInfo, ExecServerError>> {
|
||||
std::future::ready(Ok(EnvironmentInfo::local())).boxed()
|
||||
}
|
||||
}
|
||||
|
||||
struct RemoteEnvironmentInfoProvider {
|
||||
client: LazyRemoteExecServerClient,
|
||||
}
|
||||
|
||||
impl RemoteEnvironmentInfoProvider {
|
||||
fn new(client: LazyRemoteExecServerClient) -> Self {
|
||||
Self { client }
|
||||
}
|
||||
}
|
||||
|
||||
impl EnvironmentInfoProvider for RemoteEnvironmentInfoProvider {
|
||||
fn info(&self) -> BoxFuture<'_, Result<EnvironmentInfo, ExecServerError>> {
|
||||
async move { self.client.environment_info().await }.boxed()
|
||||
}
|
||||
}
|
||||
|
||||
impl Environment {
|
||||
/// Builds a test-only local environment without configured sandbox helper paths.
|
||||
pub fn default_for_tests() -> Self {
|
||||
Self {
|
||||
exec_server_url: None,
|
||||
remote_transport: None,
|
||||
info_provider: Arc::new(LocalEnvironmentInfoProvider),
|
||||
exec_backend: Arc::new(LocalProcess::default()),
|
||||
filesystem: Arc::new(LocalFileSystem::unsandboxed()),
|
||||
http_client: Arc::new(ReqwestHttpClient),
|
||||
@@ -354,6 +391,7 @@ impl Environment {
|
||||
Self {
|
||||
exec_server_url: None,
|
||||
remote_transport: None,
|
||||
info_provider: Arc::new(LocalEnvironmentInfoProvider),
|
||||
exec_backend: Arc::new(LocalProcess::default()),
|
||||
filesystem: Arc::new(LocalFileSystem::with_runtime_paths(
|
||||
local_runtime_paths.clone(),
|
||||
@@ -392,6 +430,7 @@ impl Environment {
|
||||
Self {
|
||||
exec_server_url,
|
||||
remote_transport: Some(remote_transport),
|
||||
info_provider: Arc::new(RemoteEnvironmentInfoProvider::new(client.clone())),
|
||||
exec_backend,
|
||||
filesystem,
|
||||
http_client: Arc::new(client),
|
||||
@@ -412,6 +451,11 @@ impl Environment {
|
||||
self.local_runtime_paths.as_ref()
|
||||
}
|
||||
|
||||
/// Returns environment information from the selected execution/filesystem environment.
|
||||
pub async fn info(&self) -> Result<EnvironmentInfo, ExecServerError> {
|
||||
self.info_provider.info().await
|
||||
}
|
||||
|
||||
pub fn get_exec_backend(&self) -> Arc<dyn ExecBackend> {
|
||||
Arc::clone(&self.exec_backend)
|
||||
}
|
||||
@@ -425,6 +469,23 @@ impl Environment {
|
||||
}
|
||||
}
|
||||
|
||||
impl EnvironmentInfo {
|
||||
pub(crate) fn local() -> Self {
|
||||
Self {
|
||||
shell: codex_shell_command::shell_detect::default_user_shell().into(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl From<DetectedShell> for ShellInfo {
|
||||
fn from(shell: DetectedShell) -> Self {
|
||||
Self {
|
||||
name: shell.name().to_string(),
|
||||
path: shell.shell_path.to_string_lossy().into_owned(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use std::sync::Arc;
|
||||
@@ -458,6 +519,7 @@ mod tests {
|
||||
|
||||
assert_eq!(environment.exec_server_url(), None);
|
||||
assert!(!environment.is_remote());
|
||||
assert!(environment.info().await.is_ok());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
@@ -57,6 +57,7 @@ pub use process::ExecProcessEvent;
|
||||
pub use process::ExecProcessEventReceiver;
|
||||
pub use process::StartedExecProcess;
|
||||
pub use process_id::ProcessId;
|
||||
pub use protocol::EnvironmentInfo;
|
||||
pub use protocol::ExecClosedNotification;
|
||||
pub use protocol::ExecEnvPolicy;
|
||||
pub use protocol::ExecExitedNotification;
|
||||
@@ -94,6 +95,7 @@ pub use protocol::InitializeResponse;
|
||||
pub use protocol::ProcessOutputChunk;
|
||||
pub use protocol::ReadParams;
|
||||
pub use protocol::ReadResponse;
|
||||
pub use protocol::ShellInfo;
|
||||
pub use protocol::TerminateParams;
|
||||
pub use protocol::TerminateResponse;
|
||||
pub use protocol::WriteParams;
|
||||
|
||||
@@ -19,6 +19,7 @@ pub const EXEC_TERMINATE_METHOD: &str = "process/terminate";
|
||||
pub const EXEC_OUTPUT_DELTA_METHOD: &str = "process/output";
|
||||
pub const EXEC_EXITED_METHOD: &str = "process/exited";
|
||||
pub const EXEC_CLOSED_METHOD: &str = "process/closed";
|
||||
pub const ENVIRONMENT_INFO_METHOD: &str = "environment/info";
|
||||
pub const FS_READ_FILE_METHOD: &str = "fs/readFile";
|
||||
pub const FS_WRITE_FILE_METHOD: &str = "fs/writeFile";
|
||||
pub const FS_CREATE_DIRECTORY_METHOD: &str = "fs/createDirectory";
|
||||
@@ -64,6 +65,23 @@ pub struct InitializeResponse {
|
||||
pub session_id: String,
|
||||
}
|
||||
|
||||
/// Information about an execution/filesystem environment.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct EnvironmentInfo {
|
||||
pub shell: ShellInfo,
|
||||
}
|
||||
|
||||
/// Shell detected for an execution/filesystem environment.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct ShellInfo {
|
||||
/// Stable shell name, for example `zsh`, `bash`, `powershell`, `sh`, or `cmd`.
|
||||
pub name: String,
|
||||
/// Path the exec server would use for that shell.
|
||||
pub path: String,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct ExecParams {
|
||||
|
||||
@@ -14,6 +14,7 @@ use tokio_util::task::TaskTracker;
|
||||
use crate::ExecServerRuntimePaths;
|
||||
use crate::client::http_client::PendingReqwestHttpBodyStream;
|
||||
use crate::client::http_client::ReqwestHttpRequestRunner;
|
||||
use crate::protocol::EnvironmentInfo;
|
||||
use crate::protocol::ExecParams;
|
||||
use crate::protocol::ExecResponse;
|
||||
use crate::protocol::FsCanonicalizeParams;
|
||||
@@ -147,6 +148,11 @@ impl ExecServerHandler {
|
||||
session.process().exec(params).await
|
||||
}
|
||||
|
||||
pub(crate) fn environment_info(&self) -> Result<EnvironmentInfo, JSONRPCErrorError> {
|
||||
self.require_initialized_for("environment info")?;
|
||||
Ok(EnvironmentInfo::local())
|
||||
}
|
||||
|
||||
pub(crate) async fn exec_read(
|
||||
&self,
|
||||
params: ReadParams,
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
use std::sync::Arc;
|
||||
|
||||
use crate::protocol::ENVIRONMENT_INFO_METHOD;
|
||||
use crate::protocol::EXEC_METHOD;
|
||||
use crate::protocol::EXEC_READ_METHOD;
|
||||
use crate::protocol::EXEC_TERMINATE_METHOD;
|
||||
@@ -60,6 +61,10 @@ pub(crate) fn build_router() -> RpcRouter<ExecServerHandler> {
|
||||
EXEC_METHOD,
|
||||
|handler: Arc<ExecServerHandler>, params: ExecParams| async move { handler.exec(params).await },
|
||||
);
|
||||
router.request(
|
||||
ENVIRONMENT_INFO_METHOD,
|
||||
|handler: Arc<ExecServerHandler>, _params: ()| async move { handler.environment_info() },
|
||||
);
|
||||
router.request(
|
||||
EXEC_READ_METHOD,
|
||||
|handler: Arc<ExecServerHandler>, params: ReadParams| async move {
|
||||
|
||||
Reference in New Issue
Block a user