fix(cjs-default,test): one shared <mod>.default table; cc's MCP debug logger shape and exec/execFile callback order pinned (#9500)

changelog.d/9531-cjs-default-table-mcp-logger-exec-order.md

@@ -0,0 +1,25 @@
+**One shared table for the CommonJS-default module set; claude-code's MCP
+debug logger shape and exec/execFile callback order pinned (#9500).**
+
+The set of Node builtins whose `require()` / default import hands out a
+distinct `<mod>.default` namespace was hand-maintained in five places (two of
+them inside the HIR alone, already disagreeing on `ffi`, `inspector`,
+`inspector/promises` and `wasi`); the method-call router's copy is the one
+that drifted far enough to break `require('child_process').spawn` (#9485,
+#9498). The table now lives once in `perry-dispatch`, built from one literal
+per module, and the runtime's property-read and method-call paths, the
+`default`-export resolver and the HIR's import lowering all derive from it.
+Adding a module is one line; tests pin the table's shape, the HIR's
+classification of every row, and the router test's list against the table in
+both directions. No behaviour change.
+
+Two fixtures pin the issue's other findings. The MCP debug logger's exact
+write shape — the `using`-downlevel fs wrapper, the timer/dispose buffered
+writer, the graceful-shutdown cleanup set and the `appendFileSync` → ENOENT →
+`mkdirSync(recursive)` recovery arm that is the only code creating the log
+tree — is byte-compared to node; it fails on a pre-#9491 build (the append
+did not throw, so the tree was never created) and passes on main. The
+exec/execFile callback order is pinned as what node guarantees — completion
+order, whichever API launched the child or came first; the inverted order for
+two instant `echo`s is a same-turn batch-delivery artefact node flips with
+submission order, not a rule.

crates/perry-dispatch/src/cjs_default_modules.rs

@@ -0,0 +1,151 @@
+//! The one table of Node builtins whose CommonJS `module.exports` is a
+//! namespace object distinct from the ESM namespace — the modules for which
+//! `require('<mod>')`, `import x from '<mod>'` and
+//! `process.getBuiltinModule('<mod>')` hand out a `<mod>.default` namespace
+//! whose method calls and property reads must reach the base module.
+//!
+//! #9500 (from #9485 / #9498): this knowledge used to live in FOUR
+//! hand-maintained copies — the runtime's `cjs_default_base_module` and
+//! `cjs_default_namespace_name` tables, the `cjs_default_export_value` match
+//! arm, the method-call router's own `<mod>.default → base` list, and the
+//! HIR's `is_cjs_style_native_default_import` (itself duplicated in two files
+//! that had already drifted apart: one lacked `ffi`, `inspector`,
+//! `inspector/promises` and `wasi`). The router's copy drifted far enough that
+//! `require('child_process').spawn(...)` dispatched under a name with no
+//! bucket and returned `undefined` WITHOUT SPAWNING, which is why claude-code's
+//! MCP stdio client reported `Failed to connect` (#9485). Every consumer now
+//! derives from this table: adding a module here is the whole edit.
+//!
+//! Base names are the runtime's canonical spellings (`path.posix`, not
+//! `path/posix`; `util`, not `sys`) — the alias folding happens in
+//! `normalize_native_module_name` before any lookup here.
+
+/// Builds the `(base, "<base>.default")` pairs from one literal per module, so
+/// the two spellings cannot disagree.
+macro_rules! cjs_default_namespace_modules {
+    ($($base:literal),+ $(,)?) => {
+        /// `(base module, "<base>.default")` for every Node builtin with a
+        /// distinct CommonJS default namespace. Sorted by base name.
+        pub const CJS_DEFAULT_NAMESPACE_MODULES: &[(&str, &str)] =
+            &[$(($base, concat!($base, ".default"))),+];
+    };
+}
+
+cjs_default_namespace_modules!(
+    "async_hooks",
+    "child_process",
+    "cluster",
+    "constants",
+    "dns",
+    "dns/promises",
+    "ffi",
+    "inspector",
+    "inspector/promises",
+    "module",
+    "node-pty",
+    "os",
+    "path",
+    "path.posix",
+    "path.win32",
+    "process",
+    "punycode",
+    "querystring",
+    "repl",
+    "sea",
+    "url",
+    "util",
+    "wasi",
+);
+
+/// Whether `base` (canonical spelling) has a distinct `<base>.default`
+/// CommonJS namespace.
+pub fn has_cjs_default_namespace(base: &str) -> bool {
+    cjs_default_namespace_name(base).is_some()
+}
+
+/// `base` → `"<base>.default"`, the name the CJS default namespace object is
+/// created under.
+pub fn cjs_default_namespace_name(base: &str) -> Option<&'static str> {
+    CJS_DEFAULT_NAMESPACE_MODULES
+        .iter()
+        .find(|(b, _)| *b == base)
+        .map(|(_, name)| *name)
+}
+
+/// `"<base>.default"` → `base`: the module a CJS default namespace's method
+/// calls and property reads dispatch against.
+pub fn cjs_default_base_module(namespace_name: &str) -> Option<&'static str> {
+    CJS_DEFAULT_NAMESPACE_MODULES
+        .iter()
+        .find(|(_, name)| *name == namespace_name)
+        .map(|(base, _)| *base)
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn every_row_round_trips() {
+        for (base, name) in CJS_DEFAULT_NAMESPACE_MODULES {
+            assert_eq!(*name, format!("{base}.default"));
+            assert_eq!(cjs_default_namespace_name(base), Some(*name));
+            assert_eq!(cjs_default_base_module(name), Some(*base));
+            assert!(has_cjs_default_namespace(base));
+        }
+    }
+
+    #[test]
+    fn rows_are_unique_and_sorted() {
+        let bases: Vec<&str> = CJS_DEFAULT_NAMESPACE_MODULES
+            .iter()
+            .map(|(b, _)| *b)
+            .collect();
+        let mut sorted = bases.clone();
+        sorted.sort_unstable();
+        sorted.dedup();
+        assert_eq!(bases, sorted, "keep the table sorted and duplicate-free");
+    }
+
+    #[test]
+    fn base_names_are_canonical_spellings() {
+        for (base, _) in CJS_DEFAULT_NAMESPACE_MODULES {
+            assert!(!base.starts_with("node:"), "{base}: strip the node: scheme");
+            assert!(
+                !matches!(*base, "sys" | "path/posix" | "path/win32"),
+                "{base}: an alias, not a canonical module name"
+            );
+        }
+    }
+
+    #[test]
+    fn modules_without_a_cjs_default_namespace_are_absent() {
+        for base in [
+            "fs",
+            "crypto",
+            "events",
+            "http",
+            "stream",
+            "test",
+            "child_process.default",
+        ] {
+            assert!(!has_cjs_default_namespace(base), "{base}");
+            assert_eq!(cjs_default_namespace_name(base), None, "{base}");
+        }
+        assert_eq!(cjs_default_base_module("child_process"), None);
+        assert_eq!(cjs_default_base_module("fs.default"), None);
+    }
+
+    /// The #9485 regression, pinned at the source of truth.
+    #[test]
+    fn child_process_default_maps_to_child_process() {
+        assert_eq!(
+            cjs_default_base_module("child_process.default"),
+            Some("child_process")
+        );
+        assert_eq!(
+            cjs_default_namespace_name("child_process"),
+            Some("child_process.default")
+        );
+    }
+}

crates/perry-dispatch/src/lib.rs

@@ -96,6 +96,7 @@ pub struct MethodRow {
 // re-exported below so consumers keep using `perry_dispatch::PERRY_*`.
 mod audio_table;
 mod background_table;
+mod cjs_default_modules;
 mod i18n_table;
 mod ios_table;
 mod media_table;
@@ -106,6 +107,10 @@ mod updater_table;
 
 pub use audio_table::PERRY_AUDIO_TABLE;
 pub use background_table::PERRY_BACKGROUND_TABLE;
+pub use cjs_default_modules::{
+    cjs_default_base_module, cjs_default_namespace_name, has_cjs_default_namespace,
+    CJS_DEFAULT_NAMESPACE_MODULES,
+};
 pub use i18n_table::PERRY_I18N_TABLE;
 pub use ios_table::PERRY_IOS_TABLE;
 pub use media_table::PERRY_MEDIA_TABLE;

crates/perry-hir/src/lower/lower_expr/helpers.rs

@@ -11,6 +11,10 @@ use anyhow::Result;
 use swc_ecma_ast as ast;
 
 use crate::lower_types::extract_ts_type_with_ctx;
+// #9500: one CJS-default-import predicate, derived from the shared table —
+// this file used to carry its own copy, which had drifted (no `ffi`,
+// `inspector`, `inspector/promises`, `wasi`).
+use crate::lower::module_decl::native_default_import::is_cjs_style_native_default_import;
 
 /// Whether `PERRY_GLOBAL_SCRIPT_THIS` is set — compile the program as a
 /// *global script* rather than a CJS module, so module top-level `this`
@@ -185,29 +189,6 @@ pub(crate) fn is_fetch_global_value_name(name: &str) -> bool {
     )
 }
 
-pub(crate) fn is_cjs_style_native_default_import(module_name: &str) -> bool {
-    matches!(
-        module_name,
-        "async_hooks"
-            | "child_process"
-            | "cluster"
-            | "constants"
-            | "dns"
-            | "dns/promises"
-            | "events"
-            | "module"
-            | "os"
-            | "path"
-            | "path/posix"
-            | "path/win32"
-            | "punycode"
-            | "querystring"
-            | "sys"
-            | "url"
-            | "util"
-    )
-}
-
 pub(crate) fn wrap_with_gets(property: &str, fallback: Expr, envs: Vec<LocalId>) -> Expr {
     envs.into_iter()
         .rev()

crates/perry-hir/src/lower/module_decl.rs

@@ -10,7 +10,7 @@ use super::*;
 use crate::ir::*;
 
 mod namespace;
-mod native_default_import;
+pub(super) mod native_default_import;
 pub(super) mod native_profile_import;
 mod object_literal;
 mod static_import_bindings;

crates/perry-hir/src/lower/module_decl/native_default_import.rs

@@ -10,31 +10,39 @@ pub(crate) fn canonicalize_native_import_source(raw_source: &str) -> String {
     }
 }
 
+/// Whether a native module's default import binds its CommonJS
+/// `module.exports` — a `default` property read plus a builtin-module alias
+/// for member calls — rather than the historical namespace object.
+///
+/// #9500: derived from the ONE shared table
+/// (`perry_dispatch::CJS_DEFAULT_NAMESPACE_MODULES`) that the runtime's
+/// property-read and method-call paths also consume, so the three cannot
+/// drift apart again (the HIR alone used to carry two hand-written copies of
+/// this list, and one had already lost `ffi`, `inspector`,
+/// `inspector/promises` and `wasi`). The arms below are the deliberate
+/// differences between "has a `<mod>.default` namespace at runtime" and
+/// "lowers as a CJS-style default import", each spelled out so a new table
+/// row is classified by this function automatically and only an exception
+/// needs a line here.
 pub(crate) fn is_cjs_style_native_default_import(module_name: &str) -> bool {
-    matches!(
-        module_name,
-        "async_hooks"
-            | "child_process"
-            | "cluster"
-            | "constants"
-            | "dns"
-            | "dns/promises"
-            | "events"
-            | "ffi"
-            | "inspector"
-            | "inspector/promises"
-            | "module"
-            | "os"
-            | "path"
-            | "path/posix"
-            | "path/win32"
-            | "punycode"
-            | "querystring"
-            | "sys"
-            | "url"
-            | "util"
-            | "wasi"
-    )
+    match module_name {
+        // `events`' CommonJS export is the `EventEmitter` class itself
+        // (`cjs_default_export_value("events")`), not a `<mod>.default`
+        // namespace, but the default import is still CJS-shaped.
+        "events" => true,
+        // Aliases the runtime folds before any table lookup
+        // (`normalize_native_module_name`: `sys` → `util`, `path/posix` →
+        // `path.posix`, `path/win32` → `path.win32`); the HIR sees the
+        // import's own spelling.
+        "sys" | "path/posix" | "path/win32" => true,
+        // Table rows whose default import the HIR keeps on the namespace
+        // object: `process` has its own lowering (the `source == "process"`
+        // arms in `module_decl.rs`); `node-pty`, `repl` and `sea` never took
+        // the CJS-style path — flipping them is a lowering change, not a
+        // dedup, and is left for a follow-up.
+        "node-pty" | "process" | "repl" | "sea" => false,
+        other => perry_dispatch::has_cjs_default_namespace(other),
+    }
 }
 
 pub(crate) fn node_submodule_default_export_key(module_name: &str) -> Option<&'static str> {
@@ -43,3 +51,71 @@ pub(crate) fn node_submodule_default_export_key(module_name: &str) -> Option<&'s
         _ => None,
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// Shared-table rows the HIR deliberately keeps on the namespace-object
+    /// default. Adding a row to the table classifies it CJS-style unless it
+    /// is listed here — so this list, not the table, is what a lowering
+    /// decision edits.
+    const NAMESPACE_OBJECT_DEFAULT_ROWS: &[&str] = &["node-pty", "process", "repl", "sea"];
+
+    #[test]
+    fn every_shared_table_row_is_classified() {
+        for (base, _) in perry_dispatch::CJS_DEFAULT_NAMESPACE_MODULES {
+            let expected = !NAMESPACE_OBJECT_DEFAULT_ROWS.contains(base);
+            assert_eq!(
+                is_cjs_style_native_default_import(base),
+                expected,
+                "`{base}`: shared-table row classified unexpectedly"
+            );
+        }
+        for base in NAMESPACE_OBJECT_DEFAULT_ROWS {
+            assert!(
+                perry_dispatch::has_cjs_default_namespace(base),
+                "`{base}` is listed as an exclusion but is not a shared-table row"
+            );
+        }
+    }
+
+    /// The spellings only the HIR sees (aliases + the callable-default
+    /// `events`) stay CJS-style.
+    #[test]
+    fn hir_only_spellings_are_cjs_style() {
+        for module in ["events", "sys", "path/posix", "path/win32"] {
+            assert!(is_cjs_style_native_default_import(module), "{module}");
+        }
+    }
+
+    /// #9485 / #9500: the rows one of the two former copies had lost, plus
+    /// the module the regression was found on.
+    #[test]
+    fn formerly_drifted_rows_are_cjs_style() {
+        for module in [
+            "child_process",
+            "ffi",
+            "inspector",
+            "inspector/promises",
+            "wasi",
+        ] {
+            assert!(is_cjs_style_native_default_import(module), "{module}");
+        }
+    }
+
+    #[test]
+    fn plain_esm_shaped_builtins_are_not() {
+        for module in [
+            "fs",
+            "fs/promises",
+            "crypto",
+            "http",
+            "stream",
+            "test",
+            "buffer",
+        ] {
+            assert!(!is_cjs_style_native_default_import(module), "{module}");
+        }
+    }
+}

crates/perry-runtime/Cargo.toml

@@ -278,6 +278,10 @@ node-api-host = ["dep:hex", "dep:sha2"]
 dyn-eval = ["dep:perry-parser", "dep:perry-diagnostics"]
 
 [dependencies]
+# #9500: the CJS-default module table (`<mod>.default` <-> base) is shared
+# with perry-hir through perry-dispatch, so the runtime's property-read and
+# method-call paths and the HIR's import lowering cannot drift apart.
+perry-dispatch.workspace = true
 thiserror.workspace = true
 anyhow.workspace = true
 libc.workspace = true