[codex] Fix symlinked relative import resolution

crates/perry/src/commands/compile/collect_modules.rs

@@ -18,7 +18,7 @@ use perry_transform::{
 };
 use std::collections::{HashMap, HashSet};
 use std::fs;
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 
 use crate::commands::progress::{ProgressSnapshot, VerboseProgress};
 use crate::OutputFormat;
@@ -125,11 +125,6 @@ pub(super) fn collect_js_module_imports(file_path: &std::path::Path, source: &st
         specs.push(cap[1].to_string());
     }
 
-    let parent = match file_path.parent() {
-        Some(p) => p,
-        None => return Vec::new(),
-    };
-
     let mut out: Vec<PathBuf> = Vec::new();
     let mut seen: HashSet<PathBuf> = HashSet::new();
     for spec in specs {
@@ -140,25 +135,80 @@ pub(super) fn collect_js_module_imports(file_path: &std::path::Path, source: &st
         // so the most common case (top-level package brings in submodules)
         // is covered. Inside a package's `node_modules` tree, all
         // sibling imports are relative-path anyway.
-        if !(spec.starts_with("./") || spec.starts_with("../") || spec.starts_with('/')) {
+        if !(super::resolve::is_relative_specifier(&spec) || spec.starts_with('/')) {
             continue;
         }
-        let candidate = if spec.starts_with('/') {
-            PathBuf::from(&spec)
+        let resolved_path = if spec.starts_with('/') {
+            super::resolve::resolve_absolute_import_paths(&spec)
         } else {
-            parent.join(&spec)
+            super::resolve::resolve_relative_import_paths(&spec, file_path)
         };
-        if let Some(resolved) = super::resolve::resolve_with_extensions(&candidate) {
-            if let Ok(canon) = resolved.canonicalize() {
-                if seen.insert(canon.clone()) {
-                    out.push(canon);
-                }
+        if let Some(resolved) = resolved_path {
+            if seen.insert(resolved.canonical_path.clone()) {
+                out.push(resolved.source_path);
             }
         }
     }
     out
 }
 
+struct ResolvedImport {
+    canonical_path: PathBuf,
+    source_path: PathBuf,
+    kind: ModuleKind,
+}
+
+fn cached_resolve_import_with_lexical_base(
+    import_source: &str,
+    lexical_importer_path: &Path,
+    canonical_importer_path: &Path,
+    ctx: &mut CompilationContext,
+) -> Option<ResolvedImport> {
+    // Module collection keys and reads use canonical paths, but source text
+    // relative specifiers are written against the importer path the user
+    // compiled. On platforms where /tmp is a symlink, resolving imports from
+    // the canonical /private/tmp path can make a valid "../.." edge point at a
+    // nonexistent sibling and leave imported classes unresolved.
+    let resolved = cached_resolve_import_from_base(import_source, lexical_importer_path, ctx);
+    if resolved.is_some() || lexical_importer_path == canonical_importer_path {
+        return resolved;
+    }
+    cached_resolve_import_from_base(import_source, canonical_importer_path, ctx)
+}
+
+fn cached_resolve_import_from_base(
+    import_source: &str,
+    importer_path: &Path,
+    ctx: &mut CompilationContext,
+) -> Option<ResolvedImport> {
+    let (canonical_path, kind) = cached_resolve_import(import_source, importer_path, ctx)?;
+    let source_path = source_visible_resolved_path(import_source, importer_path, &canonical_path);
+    Some(ResolvedImport {
+        canonical_path,
+        source_path,
+        kind,
+    })
+}
+
+fn source_visible_resolved_path(
+    import_source: &str,
+    importer_path: &Path,
+    canonical_path: &Path,
+) -> PathBuf {
+    let resolved = if import_source.starts_with('/') {
+        super::resolve::resolve_absolute_import_paths(import_source)
+    } else if super::resolve::is_relative_specifier(import_source) {
+        super::resolve::resolve_relative_import_paths(import_source, importer_path)
+    } else {
+        None
+    };
+
+    resolved
+        .filter(|path| path.canonical_path == canonical_path)
+        .map(|path| path.source_path)
+        .unwrap_or_else(|| canonical_path.to_path_buf())
+}
+
 /// Issue #841: Node.js submodules that Perry knows about at the
 /// resolver level (no perry-stdlib backing, no compiled-source backing)
 /// but for which we still want to provide a minimal import surface so
@@ -428,7 +478,7 @@ fn collect_module_one(
         // also walked. Template-literal / variable specifiers can't be
         // resolved statically and are skipped (V8 will surface the
         // resolution failure at runtime, same as today).
-        let transitive_paths = collect_js_module_imports(&canonical, &source);
+        let transitive_paths = collect_js_module_imports(entry_path, &source);
         ctx.js_modules.insert(
             specifier.clone(),
             JsModule {
@@ -1118,8 +1168,12 @@ fn collect_module_one(
             continue;
         }
 
-        if let Some((resolved_path, kind)) = cached_resolve_import(&import.source, &canonical, ctx)
+        if let Some(resolved) =
+            cached_resolve_import_with_lexical_base(&import.source, entry_path, &canonical, ctx)
         {
+            let resolved_path = resolved.canonical_path;
+            let source_path = resolved.source_path;
+            let kind = resolved.kind;
             import.resolved_path = Some(resolved_path.to_string_lossy().to_string());
             import.module_kind = kind;
             if let Some(sidecar) =
@@ -1241,7 +1295,7 @@ fn collect_module_one(
                             pkg_dir = dir.parent();
                         }
                     }
-                    pending.push(resolved_path);
+                    pending.push(source_path);
                 }
                 ModuleKind::Interpreted => {
                     // Perry native extension packages (ioredis, ethers, ws, mysql2, dotenv)
@@ -1354,7 +1408,7 @@ fn collect_module_one(
                         OutputFormat::Json => {}
                     }
 
-                    pending.push(resolved_path);
+                    pending.push(source_path);
                 }
                 ModuleKind::NativeRust => {
                     // Native Rust modules are handled by stdlib
@@ -1466,9 +1520,12 @@ fn collect_module_one(
                 collected: Some(ctx.native_modules.len() + ctx.js_modules.len()),
                 ..Default::default()
             });
-            if let Some((resolved_path, kind)) =
-                cached_resolve_import(src.as_str(), &canonical, ctx)
+            if let Some(resolved) =
+                cached_resolve_import_with_lexical_base(src.as_str(), entry_path, &canonical, ctx)
             {
+                let resolved_path = resolved.canonical_path;
+                let source_path = resolved.source_path;
+                let kind = resolved.kind;
                 if let Some(sidecar) =
                     declaration_sidecar_for_resolved_import(src.as_str(), &resolved_path)
                 {
@@ -1557,7 +1614,7 @@ fn collect_module_one(
                 }
 
                 match kind {
-                    ModuleKind::NativeCompiled => pending.push(resolved_path),
+                    ModuleKind::NativeCompiled => pending.push(source_path),
                     ModuleKind::Interpreted => {
                         // JS runtime (V8) support was removed, so interpreted
                         // node_modules dependencies are not followed. A direct

crates/perry/src/commands/compile/collect_modules/tests.rs

@@ -2,8 +2,8 @@
 //! Split out of `collect_modules.rs` to keep that file under the file-size gate.
 
 use super::{
-    collect_modules, env_defines_for_lowering, expand_dynamic_import_glob,
-    refuse_compile_package_native_addon,
+    collect_js_module_imports, collect_modules, env_defines_for_lowering,
+    expand_dynamic_import_glob, refuse_compile_package_native_addon,
 };
 use crate::commands::compile::{CompilationContext, DefineValue};
 use crate::commands::progress::VerboseProgress;
@@ -137,6 +137,134 @@ console.log(got);
     );
 }
 
+#[cfg(unix)]
+#[test]
+fn symlinked_entry_resolves_relative_imports_from_lexical_path() {
+    let dir = tempfile::tempdir().expect("tempdir");
+    let root = dir.path();
+    let real_parent = root.join("real-parent");
+    let alias_parent = root.join("alias-parent");
+    let real_app = real_parent.join("app");
+    let alias_app = alias_parent.join("app");
+    let alias_outside = alias_parent.join("outside");
+    let real_outside = real_parent.join("outside");
+
+    std::fs::create_dir_all(&real_app).expect("mkdir real app");
+    std::fs::create_dir_all(&alias_outside).expect("mkdir alias outside");
+    std::fs::create_dir_all(&real_outside).expect("mkdir real outside");
+    std::os::unix::fs::symlink(&real_app, &alias_app).expect("symlink app");
+
+    let dep = alias_outside.join("dep.ts");
+    let decoy_dep = real_outside.join("dep.ts");
+    let child = alias_app.join("child.ts");
+    let entry = alias_app.join("entry.ts");
+
+    std::fs::write(
+        &dep,
+        r#"
+export class ExternalCtor {
+  value: string;
+  constructor(value: string) {
+    this.value = value;
+  }
+  marker(): string {
+    return this.value;
+  }
+}
+"#,
+    )
+    .expect("write dep");
+    std::fs::write(
+        &decoy_dep,
+        r#"
+export class ExternalCtor {
+  value: string;
+  constructor(value: string) {
+    this.value = "decoy:" + value;
+  }
+  marker(): string {
+    return this.value;
+  }
+}
+"#,
+    )
+    .expect("write decoy dep");
+    std::fs::write(
+        &child,
+        r#"
+import { ExternalCtor } from "../outside/dep";
+
+export function makeValue(): string {
+  const value = new ExternalCtor("ready");
+  return value.marker();
+}
+"#,
+    )
+    .expect("write child");
+    std::fs::write(
+        &entry,
+        r#"
+import { makeValue } from "./child";
+
+console.log(makeValue());
+"#,
+    )
+    .expect("write entry");
+
+    let mut ctx = CompilationContext::new(alias_parent.to_path_buf());
+    ctx.entry_canonical = Some(entry.canonicalize().unwrap());
+    let mut visited = HashSet::new();
+    let mut next_class_id: perry_hir::ClassId = 1;
+    let progress = VerboseProgress::new(OutputFormat::Text, 0);
+
+    collect_modules(
+        &entry,
+        &mut ctx,
+        &mut visited,
+        OutputFormat::Text,
+        None,
+        &mut next_class_id,
+        false,
+        &progress,
+        None,
+    )
+    .expect("collect modules");
+
+    let dep_canonical = dep.canonicalize().expect("canonical dep");
+    let decoy_canonical = decoy_dep.canonicalize().expect("canonical decoy dep");
+    assert!(
+        ctx.native_modules.contains_key(&dep_canonical),
+        "relative imports from a symlinked entry must resolve from the lexical path; collected modules: {:?}",
+        ctx.native_modules.keys().collect::<Vec<_>>()
+    );
+    assert!(
+        !ctx.native_modules.contains_key(&decoy_canonical),
+        "source-visible lexical imports must win over the canonical sibling decoy; collected modules: {:?}",
+        ctx.native_modules.keys().collect::<Vec<_>>()
+    );
+}
+
+#[test]
+fn js_import_scan_follows_bare_dot_and_dotdot_specifiers() {
+    let dir = tempfile::tempdir().expect("tempdir");
+    let root = dir.path();
+    let app = root.join("app");
+    std::fs::create_dir_all(&app).expect("mkdir app");
+    std::fs::write(root.join("index.js"), "export const parent = 1;\n").expect("write parent");
+    std::fs::write(app.join("index.js"), "export const current = 1;\n").expect("write current");
+    let child = app.join("child.js");
+    std::fs::write(&child, "import '.';\nexport * from '..';\n").expect("write child");
+
+    let imports = collect_js_module_imports(&child, "import '.';\nexport * from '..';\n");
+    let collected = imports
+        .into_iter()
+        .map(|path| path.canonicalize().expect("canonical import"))
+        .collect::<HashSet<_>>();
+
+    assert!(collected.contains(&app.join("index.js").canonicalize().unwrap()));
+    assert!(collected.contains(&root.join("index.js").canonicalize().unwrap()));
+}
+
 fn write_compile_package_fixture(
     root: &std::path::Path,
     package_name: &str,