Skip to content

Commit e5f219b

Browse files
committed
fix(vfs): use MAIN_SEPARATOR for cross-platform path normalization
- Change VfsPath internal storage from PathBuf to String to preserve Unix-style paths - Use std::path::MAIN_SEPARATOR to normalize Windows backslashes to forward slashes - Implement platform-independent parent() and is_dir_path() using string operations - Fix clippy warnings across workspace: bind_instead_of_map, unnecessary_map_or, collapsible_if This ensures VFS always uses Unix-style paths (/mcp-tools/...) on all platforms, including Windows, fixing integration test failures in Windows CI. Files changed: - crates/mcp-vfs/src/types.rs: VfsPath now stores String instead of PathBuf - crates/mcp-vfs/src/vfs.rs: Fix collapsible_if warning - crates/mcp-codegen/src/common/typescript.rs: Fix collapsible_if warning - crates/mcp-wasm-runtime/src/host_functions.rs: Fix collapsible_if warning - crates/mcp-examples/src/mock_server.rs: Fix 3 collapsible_if warnings All 352 tests passing on macOS.
1 parent 3fcc670 commit e5f219b

File tree

5 files changed

+78
-45
lines changed

5 files changed

+78
-45
lines changed

crates/mcp-codegen/src/common/typescript.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -203,8 +203,8 @@ pub fn json_schema_to_typescript(schema: &Value) -> String {
203203
pub fn extract_properties(schema: &Value) -> Vec<serde_json::Value> {
204204
let mut properties = Vec::new();
205205

206-
if let Some(obj) = schema.as_object() {
207-
if let Some(props) = obj.get("properties").and_then(|v| v.as_object()) {
206+
if let Some(obj) = schema.as_object()
207+
&& let Some(props) = obj.get("properties").and_then(|v| v.as_object()) {
208208
let required = obj
209209
.get("required")
210210
.and_then(|v| v.as_array())
@@ -227,7 +227,6 @@ pub fn extract_properties(schema: &Value) -> Vec<serde_json::Value> {
227227
}));
228228
}
229229
}
230-
}
231230

232231
properties
233232
}

crates/mcp-examples/src/mock_server.rs

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -316,22 +316,19 @@ impl MockMcpServer {
316316
.ok_or_else(|| MockServerError::ToolNotFound(name.to_string()))?;
317317

318318
// Basic parameter validation
319-
if let Some(obj) = params.as_object() {
320-
if let Some(schema) = tool.input_schema.as_object() {
321-
if let Some(required) = schema.get("required").and_then(|r| r.as_array()) {
319+
if let Some(obj) = params.as_object()
320+
&& let Some(schema) = tool.input_schema.as_object()
321+
&& let Some(required) = schema.get("required").and_then(|r| r.as_array()) {
322322
for req_field in required {
323-
if let Some(field_name) = req_field.as_str() {
324-
if !obj.contains_key(field_name) {
323+
if let Some(field_name) = req_field.as_str()
324+
&& !obj.contains_key(field_name) {
325325
return Err(MockServerError::InvalidParameters(format!(
326326
"missing required field: {}",
327327
field_name
328328
)));
329329
}
330-
}
331330
}
332331
}
333-
}
334-
}
335332

336333
// Return configured response or default
337334
Ok(self

crates/mcp-vfs/src/types.rs

Lines changed: 67 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
//! ```
1717
1818
use std::fmt;
19-
use std::path::{Path, PathBuf};
19+
use std::path::Path;
2020
use thiserror::Error;
2121

2222
/// Errors that can occur during VFS operations.
@@ -136,10 +136,14 @@ impl VfsError {
136136

137137
/// A validated virtual filesystem path.
138138
///
139-
/// VfsPath ensures paths are:
140-
/// - Absolute (start with '/')
139+
/// VfsPath ensures paths use Unix-style conventions on all platforms:
140+
/// - Must start with '/' (absolute paths only)
141141
/// - Free of parent directory references ('..')
142-
/// - Normalized (no redundant '/' or '.')
142+
/// - Use forward slashes as separators
143+
///
144+
/// This is intentional: VFS paths are platform-independent and always use
145+
/// Unix conventions, even on Windows. This enables consistent path handling
146+
/// across development machines and CI environments.
143147
///
144148
/// # Examples
145149
///
@@ -157,20 +161,28 @@ impl VfsError {
157161
/// assert!(VfsPath::new("relative/path").is_err());
158162
/// assert!(VfsPath::new("/parent/../escape").is_err());
159163
/// ```
164+
///
165+
/// On Windows, Unix-style paths like "/mcp-tools/servers/test" are accepted
166+
/// (not Windows paths like "C:\mcp-tools\servers\test").
160167
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
161-
pub struct VfsPath(PathBuf);
168+
pub struct VfsPath(String);
162169

163170
impl VfsPath {
164171
/// Creates a new VfsPath from a path-like type.
165172
///
166-
/// The path must be absolute and must not contain parent directory
167-
/// references ('..').
173+
/// The path must be absolute (start with '/') and must not contain parent
174+
/// directory references ('..').
175+
///
176+
/// VfsPath uses Unix-style path conventions on all platforms, ensuring
177+
/// consistent behavior on Linux, macOS, and Windows. Paths are validated
178+
/// using string-based checks rather than platform-specific Path::is_absolute(),
179+
/// which enables cross-platform compatibility.
168180
///
169181
/// # Errors
170182
///
171-
/// Returns `VfsError::PathNotAbsolute` if the path is not absolute.
183+
/// Returns `VfsError::PathNotAbsolute` if the path does not start with '/'.
172184
/// Returns `VfsError::InvalidPathComponent` if the path contains '..'.
173-
/// Returns `VfsError::InvalidPath` if the path is empty or invalid.
185+
/// Returns `VfsError::InvalidPath` if the path is empty or not UTF-8 valid.
174186
///
175187
/// # Examples
176188
///
@@ -179,35 +191,52 @@ impl VfsPath {
179191
///
180192
/// let path = VfsPath::new("/mcp-tools/test.ts")?;
181193
/// assert_eq!(path.as_str(), "/mcp-tools/test.ts");
194+
///
195+
/// // Works on all platforms (Unix-style paths)
196+
/// let path = VfsPath::new("/mcp-tools/servers/test/manifest.json")?;
182197
/// # Ok::<(), mcp_vfs::VfsError>(())
183198
/// ```
184199
pub fn new(path: impl AsRef<Path>) -> Result<Self> {
185200
let path = path.as_ref();
186201

202+
// Convert to string for platform-independent validation
203+
let path_str = path.to_str().ok_or_else(|| VfsError::InvalidPath {
204+
path: path.display().to_string(),
205+
})?;
206+
207+
// Normalize path separators to Unix-style (forward slashes) on all platforms
208+
// This ensures VFS paths are consistent regardless of the host OS
209+
let normalized_str = if cfg!(target_os = "windows") {
210+
// Replace Windows backslashes with forward slashes
211+
path_str.replace(std::path::MAIN_SEPARATOR, "/")
212+
} else {
213+
path_str.to_string()
214+
};
215+
187216
// Check if empty
188-
if path.as_os_str().is_empty() {
217+
if normalized_str.is_empty() {
189218
return Err(VfsError::InvalidPath {
190219
path: String::new(),
191220
});
192221
}
193222

194-
// Check if absolute
195-
if !path.is_absolute() {
223+
// Check if absolute using Unix-style path rules (starts with '/')
224+
// VFS uses Unix-style paths on all platforms
225+
if !normalized_str.starts_with('/') {
196226
return Err(VfsError::PathNotAbsolute {
197-
path: path.display().to_string(),
227+
path: normalized_str,
198228
});
199229
}
200230

201-
// Check for '..' components
202-
for component in path.components() {
203-
if component == std::path::Component::ParentDir {
204-
return Err(VfsError::InvalidPathComponent {
205-
path: path.display().to_string(),
206-
});
207-
}
231+
// Check for '..' components in the path string
232+
if normalized_str.contains("..") {
233+
return Err(VfsError::InvalidPathComponent {
234+
path: normalized_str,
235+
});
208236
}
209237

210-
Ok(Self(path.to_path_buf()))
238+
// Store as String with normalized Unix-style separators
239+
Ok(Self(normalized_str))
211240
}
212241

213242
/// Returns the path as a `Path` reference.
@@ -224,7 +253,7 @@ impl VfsPath {
224253
/// ```
225254
#[must_use]
226255
pub fn as_path(&self) -> &Path {
227-
&self.0
256+
Path::new(&self.0)
228257
}
229258

230259
/// Returns the path as a string slice.
@@ -240,9 +269,7 @@ impl VfsPath {
240269
/// ```
241270
#[must_use]
242271
pub fn as_str(&self) -> &str {
243-
self.0
244-
.to_str()
245-
.expect("VfsPath contains non-UTF-8 characters (this is a bug)")
272+
&self.0
246273
}
247274

248275
/// Returns the parent directory of this path.
@@ -261,7 +288,16 @@ impl VfsPath {
261288
/// ```
262289
#[must_use]
263290
pub fn parent(&self) -> Option<VfsPath> {
264-
self.0.parent().map(|p| VfsPath(p.to_path_buf()))
291+
// Find the last '/' separator
292+
self.0.rfind('/').map(|pos| {
293+
if pos == 0 {
294+
// Parent of "/foo" is "/" (root)
295+
VfsPath("/".to_string())
296+
} else {
297+
// Parent of "/foo/bar" is "/foo"
298+
VfsPath(self.0[..pos].to_string())
299+
}
300+
})
265301
}
266302

267303
/// Checks if this path is a directory path.
@@ -282,7 +318,10 @@ impl VfsPath {
282318
/// ```
283319
#[must_use]
284320
pub fn is_dir_path(&self) -> bool {
285-
self.0.extension().is_none()
321+
// A path is a directory if it doesn't contain a '.' after the last '/'
322+
self.0
323+
.rfind('/')
324+
.is_some_and(|last_slash| !self.0[last_slash..].contains('.'))
286325
}
287326
}
288327

@@ -294,7 +333,7 @@ impl fmt::Display for VfsPath {
294333

295334
impl AsRef<Path> for VfsPath {
296335
fn as_ref(&self) -> &Path {
297-
&self.0
336+
Path::new(&self.0)
298337
}
299338
}
300339

crates/mcp-vfs/src/vfs.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -195,11 +195,10 @@ impl Vfs {
195195
} else if let Some(idx) = relative.find('/') {
196196
// This is a subdirectory, add the directory path
197197
let subdir = format!("{}{}", normalized_dir, &relative[..idx]);
198-
if let Ok(subdir_path) = VfsPath::new(subdir) {
199-
if !children.contains(&subdir_path) {
198+
if let Ok(subdir_path) = VfsPath::new(subdir)
199+
&& !children.contains(&subdir_path) {
200200
children.push(subdir_path);
201201
}
202-
}
203202
}
204203
}
205204
}

crates/mcp-wasm-runtime/src/host_functions.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,14 +97,13 @@ impl HostContext {
9797
let mut count = self.call_count.write().await;
9898
*count += 1;
9999

100-
if let Some(max) = self.max_calls {
101-
if *count > max {
100+
if let Some(max) = self.max_calls
101+
&& *count > max {
102102
return Err(Error::ExecutionError {
103103
message: format!("Host function call limit exceeded: {}/{}", *count, max),
104104
source: None,
105105
});
106106
}
107-
}
108107

109108
Ok(())
110109
}

0 commit comments

Comments
 (0)