From 6f3d83e8646ce0adf845dfba9ae95f8bba69e4ad Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Wed, 25 Mar 2026 15:04:58 -0600 Subject: [PATCH 1/3] =?UTF-8?q?fix:=20normalize=20paths=20in=20native=20re?= =?UTF-8?q?solver=20for=20.js=20=E2=86=92=20.ts=20extension=20remap?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds clean_path() to properly resolve .. components before extension probing. Also updates normalize_path() to use clean_path() internally. The JS-side defense-in-depth (path.normalize + remapJsToTs) was already in place; this fixes the root cause in the Rust native resolver. Fixes #592 Impact: 3 functions changed, 6 affected --- .../codegraph-core/src/import_resolution.rs | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/crates/codegraph-core/src/import_resolution.rs b/crates/codegraph-core/src/import_resolution.rs index 69480151..65ec6f58 100644 --- a/crates/codegraph-core/src/import_resolution.rs +++ b/crates/codegraph-core/src/import_resolution.rs @@ -10,10 +10,27 @@ fn file_exists(path: &str, known: Option<&HashSet>) -> bool { known.map_or_else(|| Path::new(path).exists(), |set| set.contains(path)) } +/// Resolve `.` and `..` components in a path without touching the filesystem. +/// Unlike `PathBuf::components().collect()`, this properly collapses `..` by +/// popping the previous component from the result. +fn clean_path(p: &Path) -> PathBuf { + let mut result = PathBuf::new(); + for c in p.components() { + match c { + std::path::Component::ParentDir => { + result.pop(); + } + std::path::Component::CurDir => {} + _ => result.push(c), + } + } + result +} + /// Normalize a path to use forward slashes and clean `.` / `..` segments /// (cross-platform consistency). fn normalize_path(p: &str) -> String { - let cleaned: PathBuf = Path::new(p).components().collect(); + let cleaned = clean_path(Path::new(p)); cleaned.display().to_string().replace('\\', "/") } @@ -111,7 +128,7 @@ fn resolve_import_path_inner( // Relative import — normalize immediately to remove `.` / `..` segments let dir = Path::new(from_file).parent().unwrap_or(Path::new("")); - let resolved: PathBuf = dir.join(import_source).components().collect(); + let resolved = clean_path(&dir.join(import_source)); let resolved_str = resolved.display().to_string().replace('\\', "/"); // .js → .ts remap From 5e4079294ab1fc0a8816afb8532725243a4b7655 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Wed, 25 Mar 2026 15:15:23 -0600 Subject: [PATCH 2/3] fix: add clean_path tests, doc comment, and un-skip parity test (#600) - Add #[cfg(test)] module with unit tests for clean_path covering parent dir collapse, cur dir skip, absolute root, mixed segments, and the known excess-parent-dir limitation. - Document the silent-drop behavior of leading .. on empty base in the clean_path doc comment. - Un-skip the 'resolves parent directory traversal' parity test now that the native engine properly normalizes paths. Impact: 5 functions changed, 0 affected --- .../codegraph-core/src/import_resolution.rs | 52 +++++++++++++++++++ tests/resolution/parity.test.ts | 3 +- 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/crates/codegraph-core/src/import_resolution.rs b/crates/codegraph-core/src/import_resolution.rs index 65ec6f58..3a602585 100644 --- a/crates/codegraph-core/src/import_resolution.rs +++ b/crates/codegraph-core/src/import_resolution.rs @@ -13,6 +13,12 @@ fn file_exists(path: &str, known: Option<&HashSet>) -> bool { /// Resolve `.` and `..` components in a path without touching the filesystem. /// Unlike `PathBuf::components().collect()`, this properly collapses `..` by /// popping the previous component from the result. +/// +/// NOTE: if the path begins with more `..` components than there are preceding +/// components to pop (e.g. a purely relative `../../foo`), the excess `..` +/// components are silently dropped. This function is therefore only correct +/// when called on paths that have already been joined to a base directory with +/// sufficient depth. fn clean_path(p: &Path) -> PathBuf { let mut result = PathBuf::new(); for c in p.components() { @@ -263,3 +269,49 @@ pub fn resolve_imports_batch( }) .collect() } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn clean_path_collapses_parent_dirs() { + assert_eq!( + clean_path(Path::new("src/cli/commands/../../domain/graph/builder.js")), + PathBuf::from("src/domain/graph/builder.js") + ); + } + + #[test] + fn clean_path_skips_cur_dir() { + assert_eq!( + clean_path(Path::new("src/./foo.ts")), + PathBuf::from("src/foo.ts") + ); + } + + #[test] + fn clean_path_handles_absolute_root() { + assert_eq!( + clean_path(Path::new("/src/../foo.ts")), + PathBuf::from("/foo.ts") + ); + } + + #[test] + fn clean_path_mixed_segments() { + assert_eq!( + clean_path(Path::new("a/b/../c/./d/../e.js")), + PathBuf::from("a/c/e.js") + ); + } + + #[test] + fn clean_path_excess_parent_dirs_silently_dropped() { + // Documents the known limitation: excess leading `..` are dropped + assert_eq!( + clean_path(Path::new("../../foo")), + PathBuf::from("foo") + ); + } +} diff --git a/tests/resolution/parity.test.ts b/tests/resolution/parity.test.ts index 6c036ee0..417a4041 100644 --- a/tests/resolution/parity.test.ts +++ b/tests/resolution/parity.test.ts @@ -90,8 +90,7 @@ describe.skipIf(!hasNative)('Import resolution parity', () => { assertParity(path.join(rootDir, 'index.js'), 'lodash', noAliases); }); - // Known native engine limitation: does not normalize ../ or append extensions - it.skip('resolves parent directory traversal', () => { + it('resolves parent directory traversal', () => { // Create a temporary nested structure const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-parity-')); const subDir = path.join(tmpDir, 'sub'); From 5b5827330715c7fe7f2972d245db8d3cdd958afe Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Wed, 25 Mar 2026 15:19:44 -0600 Subject: [PATCH 3/3] fix: re-skip parity test until native binary is published (#600) The parity test requires the native binary to include the clean_path fix, but CI tests run against the pre-built binary. Re-skip with an updated comment explaining when to un-skip. --- tests/resolution/parity.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/resolution/parity.test.ts b/tests/resolution/parity.test.ts index 417a4041..4d7877e3 100644 --- a/tests/resolution/parity.test.ts +++ b/tests/resolution/parity.test.ts @@ -90,7 +90,8 @@ describe.skipIf(!hasNative)('Import resolution parity', () => { assertParity(path.join(rootDir, 'index.js'), 'lodash', noAliases); }); - it('resolves parent directory traversal', () => { + // Un-skip once the native binary is published with the clean_path fix (#600) + it.skip('resolves parent directory traversal', () => { // Create a temporary nested structure const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-parity-')); const subDir = path.join(tmpDir, 'sub');