Compare commits

..

3 Commits

Author SHA1 Message Date
shijie-openai
620164e511 Covert to use absolutePathBuf for mcp cwd 2026-01-20 20:17:39 -08:00
shijie-openai
8a7ee646c5 Also need to expand tilda 2026-01-20 20:17:39 -08:00
shijie-openai
4371913278 fix: first pass at fixing the path issue 2026-01-20 20:17:39 -08:00
11 changed files with 94 additions and 72 deletions

View File

@@ -1 +0,0 @@
8.5.1

View File

@@ -18,7 +18,8 @@ In the codex-rs folder where the rust code lives:
Run `just fmt` (in `codex-rs` directory) automatically after making Rust code changes; do not ask for approval to run it. Before finalizing a change to `codex-rs`, run `just fix -p <project>` (in `codex-rs` directory) to fix any linter issues in the code. Prefer scoping with `-p` to avoid slow workspacewide Clippy builds; only run `just fix` without `-p` if you changed shared crates. Additionally, run the tests:
1. Run the test for the specific project that was changed. For example, if changes were made in `codex-rs/tui`, run `cargo test -p codex-tui`.
2. Once those pass, if any changes were made in common, core, or protocol, run the complete test suite with `cargo test --all-features`. project-specific or individual tests can be run without asking the user, but do ask the user before running the complete test suite.
2. Once those pass, if any changes were made in common, core, or protocol, run the complete test suite with `cargo test --all-features`.
When running interactively, ask the user before running `just fix` to finalize. `just fmt` does not require approval. project-specific or individual tests can be run without asking the user, but do ask the user before running the complete test suite.
## TUI style conventions

View File

@@ -113,9 +113,7 @@ impl<A: AuthProvider> ResponsesWebsocketClient<A> {
extra_headers: HeaderMap,
turn_state: Option<Arc<OnceLock<String>>>,
) -> Result<ResponsesWebsocketConnection, ApiError> {
let ws_url = self
.provider
.websocket_url_for_path("responses")
let ws_url = Url::parse(&self.provider.url_for_path("responses"))
.map_err(|err| ApiError::Stream(format!("failed to build websocket URL: {err}")))?;
let mut headers = self.provider.headers.clone();

View File

@@ -6,7 +6,6 @@ use http::Method;
use http::header::HeaderMap;
use std::collections::HashMap;
use std::time::Duration;
use url::Url;
/// Wire-level APIs supported by a `Provider`.
#[derive(Debug, Clone, PartialEq, Eq)]
@@ -106,19 +105,6 @@ impl Provider {
self.base_url.to_ascii_lowercase().contains("openai.azure.")
|| matches_azure_responses_base_url(&self.base_url)
}
pub fn websocket_url_for_path(&self, path: &str) -> Result<Url, url::ParseError> {
let mut url = Url::parse(&self.url_for_path(path))?;
let scheme = match url.scheme() {
"http" => "ws",
"https" => "wss",
"ws" | "wss" => return Ok(url),
_ => return Ok(url),
};
let _ = url.set_scheme(scheme);
Ok(url)
}
}
fn matches_azure_responses_base_url(base_url: &str) -> bool {

View File

@@ -109,9 +109,6 @@
"remote_models": {
"type": "boolean"
},
"responses_websockets": {
"type": "boolean"
},
"shell_snapshot": {
"type": "boolean"
},
@@ -604,9 +601,6 @@
"remote_models": {
"type": "boolean"
},
"responses_websockets": {
"type": "boolean"
},
"shell_snapshot": {
"type": "boolean"
},
@@ -1110,7 +1104,11 @@
},
"cwd": {
"default": null,
"type": "string"
"allOf": [
{
"$ref": "#/definitions/AbsolutePathBuf"
}
]
},
"disabled_tools": {
"default": null,

View File

@@ -1363,12 +1363,6 @@ impl Config {
|| cfg.sandbox_mode.is_some();
let mut model_providers = built_in_model_providers();
if features.enabled(Feature::ResponsesWebsockets)
&& let Some(provider) = model_providers.get_mut("openai")
&& provider.is_openai()
{
provider.wire_api = crate::model_provider_info::WireApi::ResponsesWebsocket;
}
// Merge user-defined providers into the built-in list.
for (key, provider) in cfg.model_providers.into_iter() {
model_providers.entry(key).or_insert(provider);
@@ -2517,30 +2511,6 @@ profile = "project"
Ok(())
}
#[test]
fn responses_websockets_feature_updates_openai_provider() -> std::io::Result<()> {
let codex_home = TempDir::new()?;
let mut entries = BTreeMap::new();
entries.insert("responses_websockets".to_string(), true);
let cfg = ConfigToml {
features: Some(crate::features::FeaturesToml { entries }),
..Default::default()
};
let config = Config::load_from_base_config_with_overrides(
cfg,
ConfigOverrides::default(),
codex_home.path().to_path_buf(),
)?;
assert_eq!(
config.model_provider.wire_api,
crate::model_provider_info::WireApi::ResponsesWebsocket
);
Ok(())
}
#[test]
fn config_honors_explicit_file_oauth_store_mode() -> std::io::Result<()> {
let codex_home = TempDir::new()?;
@@ -2894,7 +2864,7 @@ ZIG_VAR = "3"
async fn replace_mcp_servers_serializes_cwd() -> anyhow::Result<()> {
let codex_home = TempDir::new()?;
let cwd_path = PathBuf::from("/tmp/codex-mcp");
let cwd_path = AbsolutePathBuf::from_absolute_path("/tmp/codex-mcp").expect("expected cwd");
let servers = BTreeMap::from([(
"docs".to_string(),
McpServerConfig {
@@ -2931,7 +2901,10 @@ ZIG_VAR = "3"
let docs = loaded.get("docs").expect("docs entry");
match &docs.transport {
McpServerTransportConfig::Stdio { cwd, .. } => {
assert_eq!(cwd.as_deref(), Some(Path::new("/tmp/codex-mcp")));
assert_eq!(
cwd.as_ref().map(AbsolutePathBuf::as_path),
Some(Path::new("/tmp/codex-mcp"))
);
}
other => panic!("unexpected transport {other:?}"),
}

View File

@@ -10,10 +10,10 @@ use codex_utils_absolute_path::AbsolutePathBuf;
use std::collections::BTreeMap;
use std::collections::HashMap;
use std::fmt;
use std::path::PathBuf;
use std::time::Duration;
use wildmatch::WildMatchPattern;
use dirs::home_dir;
use schemars::JsonSchema;
use serde::Deserialize;
use serde::Deserializer;
@@ -87,7 +87,7 @@ pub(crate) struct RawMcpServerConfig {
#[serde(default)]
pub env_vars: Option<Vec<String>>,
#[serde(default)]
pub cwd: Option<PathBuf>,
pub cwd: Option<AbsolutePathBuf>,
pub http_headers: Option<HashMap<String, String>>,
#[serde(default)]
pub env_http_headers: Option<HashMap<String, String>>,
@@ -156,7 +156,7 @@ impl<'de> Deserialize<'de> for McpServerConfig {
throw_if_set("stdio", "http_headers", raw.http_headers.as_ref())?;
throw_if_set("stdio", "env_http_headers", raw.env_http_headers.as_ref())?;
McpServerTransportConfig::Stdio {
command,
command: expand_tilde_string(command),
args: raw.args.clone().unwrap_or_default(),
env: raw.env.clone(),
env_vars: raw.env_vars.clone().unwrap_or_default(),
@@ -194,6 +194,26 @@ const fn default_enabled() -> bool {
true
}
fn expand_tilde_string(value: String) -> String {
if cfg!(target_os = "windows") {
return value;
}
let Some(home) = home_dir() else {
return value;
};
if value == "~" {
return home.to_string_lossy().into_owned();
}
if let Some(rest) = value.strip_prefix("~/") {
return home.join(rest).to_string_lossy().into_owned();
}
value
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema)]
#[serde(untagged, deny_unknown_fields, rename_all = "snake_case")]
pub enum McpServerTransportConfig {
@@ -207,7 +227,7 @@ pub enum McpServerTransportConfig {
#[serde(default, skip_serializing_if = "Vec::is_empty")]
env_vars: Vec<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
cwd: Option<PathBuf>,
cwd: Option<AbsolutePathBuf>,
},
/// https://modelcontextprotocol.io/specification/2025-06-18/basic/transports#streamable-http
StreamableHttp {
@@ -759,6 +779,7 @@ pub enum Personality {
#[cfg(test)]
mod tests {
use super::*;
use codex_utils_absolute_path::AbsolutePathBufGuard;
use pretty_assertions::assert_eq;
#[test]
@@ -871,7 +892,44 @@ mod tests {
args: vec![],
env: None,
env_vars: Vec::new(),
cwd: Some(PathBuf::from("/tmp")),
cwd: Some(
AbsolutePathBuf::from_absolute_path("/tmp").expect("expected absolute mcp cwd"),
),
}
);
}
#[test]
fn deserialize_stdio_command_server_config_expands_tilde_cwd() {
let base_dir = std::env::temp_dir();
let _guard = AbsolutePathBufGuard::new(&base_dir);
let cfg: McpServerConfig = toml::from_str(
r#"
command = "echo"
cwd = "~/tmp"
"#,
)
.expect("should deserialize command config with tilde cwd");
let expected_cwd = if cfg!(target_os = "windows") {
AbsolutePathBuf::resolve_path_against_base("~/tmp", &base_dir)
.expect("expected absolute mcp cwd")
} else {
let Some(home) = home_dir() else {
return;
};
AbsolutePathBuf::from_absolute_path(home.join("tmp"))
.expect("expected absolute mcp cwd")
};
assert_eq!(
cfg.transport,
McpServerTransportConfig::Stdio {
command: "echo".to_string(),
args: vec![],
env: None,
env_vars: Vec::new(),
cwd: Some(expected_cwd),
}
);
}

View File

@@ -107,8 +107,6 @@ pub enum Feature {
Steer,
/// Enable collaboration modes (Plan, Pair Programming, Execute).
CollaborationModes,
/// Use the Responses API WebSocket transport for OpenAI by default.
ResponsesWebsockets,
}
impl Feature {
@@ -463,10 +461,4 @@ pub const FEATURES: &[FeatureSpec] = &[
stage: Stage::Beta,
default_enabled: false,
},
FeatureSpec {
id: Feature::ResponsesWebsockets,
key: "responses_websockets",
stage: Stage::Beta,
default_enabled: false,
},
];

View File

@@ -865,6 +865,7 @@ async fn make_rmcp_client(
} => {
let command_os: OsString = command.into();
let args_os: Vec<OsString> = args.into_iter().map(Into::into).collect();
let cwd = cwd.map(codex_utils_absolute_path::AbsolutePathBuf::into_path_buf);
RmcpClient::new_stdio_client(command_os, args_os, env, &env_vars, cwd)
.await
.map_err(|err| StartupOutcomeError::from(anyhow!(err)))

View File

@@ -1,6 +1,7 @@
use std::collections::HashMap;
use std::ffi::OsString;
use std::io;
use std::path::Path;
use std::path::PathBuf;
use std::process::Stdio;
use std::sync::Arc;
@@ -109,6 +110,7 @@ impl RmcpClient {
env_vars: &[String],
cwd: Option<PathBuf>,
) -> io::Result<Self> {
let program = resolve_relative_program(program, cwd.as_deref());
let program_name = program.to_string_lossy().into_owned();
// Build environment for program resolution and subprocess
@@ -435,6 +437,20 @@ impl RmcpClient {
}
}
fn resolve_relative_program(program: OsString, cwd: Option<&Path>) -> OsString {
let Some(cwd) = cwd else {
return program;
};
let program_path = Path::new(&program);
let has_separator = program_path.components().count() > 1;
if program_path.is_relative() && has_separator {
return cwd.join(program_path).into_os_string();
}
program
}
async fn create_oauth_transport_and_runtime(
server_name: &str,
url: &str,

View File

@@ -64,7 +64,7 @@ There are multiple submission paths, but they share the same core rules:
3. Expands `/prompts:` custom prompts:
- Named args use key=value parsing.
- Numeric args use positional parsing for `$1..$9` and `$ARGUMENTS`.
The expansion preserves text elements and yields the final submission payload.
The expansion preserves text elements and yields the final submission payload.
4. Prunes attachments so only placeholders that survive expansion are sent.
5. Clears pending pastes on success and suppresses submission if the final text is empty and there
are no attachments.