From 4b4f85004ab10871ba637a12fc64e7263a5af8d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 2 Sep 2026 09:19:18 +0200 Subject: [PATCH 1/2] =?UTF-8?q?test(9485):=20gap=20fixture=20=E2=80=94=20f?= =?UTF-8?q?used=20member=20call=20on=20a=20CJS-default=20native=20namespac?= =?UTF-8?q?e?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `const cp = require('child_process'); cp.spawn(...)` dispatches under the module name `child_process.default`, which the runtime's method-call router does not normalize, so the call returns `undefined` without spawning. The two-step forms (`const f = cp.spawn; f(...)`) are unaffected — the property-read path already normalizes through `cjs_default_base_module`. Fails on origin/main; refs #9485 --- ..._9485_cjs_default_namespace_method_call.ts | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 test-files/test_gap_9485_cjs_default_namespace_method_call.ts diff --git a/test-files/test_gap_9485_cjs_default_namespace_method_call.ts b/test-files/test_gap_9485_cjs_default_namespace_method_call.ts new file mode 100644 index 00000000000..388e7ae87e9 --- /dev/null +++ b/test-files/test_gap_9485_cjs_default_namespace_method_call.ts @@ -0,0 +1,60 @@ +// #9485: a METHOD CALL on the CJS-default namespace object that +// `require('child_process')` (and `process.getBuiltinModule('child_process')`) +// returns dispatched under the module name `"child_process.default"`. The +// runtime's method-call router normalized only a hand-maintained subset of the +// `.default` names — `child_process.default` was not on it — so the call +// found no dispatch bucket and returned `undefined` WITHOUT SPAWNING ANYTHING. +// +// The asymmetry is the tell: the two-step forms (`const f = cp.spawn; f(...)`, +// `cp.spawn.call(...)`) always worked, because the property-READ path resolves +// through the canonical `cjs_default_base_module` table. Only the fused +// member-call form was broken. +// +// That is exactly the shape `cross-spawn` uses +// (`const cp = require('child_process'); cp.spawn(command, args, options)`), +// which is what the MCP SDK's StdioClientTransport spawns servers with — so +// claude-code under perry reported `✗ Failed to connect` for every stdio MCP +// server while never launching a child at all. +import { createRequire } from "node:module"; + +const req = createRequire(import.meta.url); + +const cp: any = req("child_process"); +const dns: any = req("dns"); +const os: any = req("os"); + +// ── 1. the regression: fused member call on the CJS-default namespace ── +const sync = cp.spawnSync("/bin/echo", ["spawnSync-member"], { encoding: "utf8" }); +console.log("spawnSync member typeof:", typeof sync); +console.log("spawnSync member stdout:", String(sync.stdout).trim()); + +// A computed member call takes the same route. +const syncComputed = cp["spawnSync"]("/bin/echo", ["spawnSync-computed"], { encoding: "utf8" }); +console.log("spawnSync computed stdout:", String(syncComputed.stdout).trim()); + +console.log("execSync member stdout:", String(cp.execSync("/bin/echo execSync-member")).trim()); + +// ── 2. control: the two-step forms were never broken ── +const hoisted = cp.spawnSync; +console.log("spawnSync hoisted stdout:", String(hoisted("/bin/echo", ["hoisted"], { encoding: "utf8" }).stdout).trim()); + +// ── 3. sibling `.default` namespaces that shared the gap ── +console.log("dns.getServers isArray:", Array.isArray(dns.getServers())); + +// ── 4. a namespace that was already on the list, as a negative control ── +console.log("os.platform is string:", typeof os.platform() === "string"); + +// ── 5. the streaming spawn the MCP client actually uses ── +const child = cp.spawn("/bin/echo", ["spawn-streams"]); +console.log("spawn member typeof:", typeof child); +console.log("spawn member pid is number:", typeof child.pid === "number"); +await new Promise((resolve) => { + let buf = ""; + child.stdout.on("data", (chunk: any) => { + buf += chunk.toString(); + }); + child.on("exit", () => { + console.log("spawn member stdout:", buf.trim()); + resolve(); + }); +}); From c1e776bf25f283ab0533ee84b10e9539cdfe1078 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 2 Sep 2026 09:26:44 +0200 Subject: [PATCH 2/2] fix(runtime): a method call on a CJS-default native namespace dispatches to its base module (#9485) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `dispatch_native_module_method` carried its own hand-maintained copy of the `.default` → base-module table, and it had drifted from the canonical `cjs_default_base_module` the property-read path uses. `child_process.default` was missing, so `require('child_process').spawn(...)` — cross-spawn's exact shape, and therefore the MCP SDK's StdioClientTransport — found no dispatch bucket and returned `undefined` without spawning anything. The two-step forms (`const f = cp.spawn; f(...)`) were unaffected, which is what made the failure look module-specific rather than structural. The router now falls through to the canonical table instead of duplicating it. The normalization is split into `normalize_dispatch_module_name` so the agreement between the two paths is unit-testable: every `.default` name must normalize to the same base the property-read path picks, and must reach the same dispatch bucket. Also fixed by the same fall-through: `constants.default`, `dns.default`, `dns/promises.default`, `inspector.default`, `inspector/promises.default`, `module.default`, `repl.default`, `sea.default`, `wasi.default`. Fixes #9485 --- .../9485-cjs-default-namespace-method-call.md | 35 ++++ .../src/object/native_module_dispatch.rs | 175 ++++++++++++++++-- 2 files changed, 190 insertions(+), 20 deletions(-) create mode 100644 changelog.d/9485-cjs-default-namespace-method-call.md diff --git a/changelog.d/9485-cjs-default-namespace-method-call.md b/changelog.d/9485-cjs-default-namespace-method-call.md new file mode 100644 index 00000000000..0f147d18ca9 --- /dev/null +++ b/changelog.d/9485-cjs-default-namespace-method-call.md @@ -0,0 +1,35 @@ +**`require('child_process').spawn(...)` now actually spawns.** A method call on +the CJS-default namespace object that `require()` / +`process.getBuiltinModule()` returns dispatched under the module name +`child_process.default`, which the runtime's method-call router did not +normalize — so the call found no dispatch bucket and returned `undefined` +without launching a process, throwing nothing. + +The tell was the asymmetry. `const f = cp.spawn; f(...)`, `cp.spawn.call(...)` +and `(0, cp.spawn)(...)` all worked, because the property-READ path resolves +through `cjs_default_base_module`. Only the fused member-call form +(`cp.spawn(...)`, `cp['spawn'](...)`) was broken, because the router carried a +second, hand-maintained copy of that table — and it had drifted. +`child_process.default`, `constants.default`, `dns.default`, +`dns/promises.default`, `inspector.default`, `inspector/promises.default`, +`module.default`, `repl.default`, `sea.default` and `wasi.default` were all +missing from it, while `os.default`, `path.default`, `util.default` and friends +were present — which is why the failure looked module-specific rather than +structural. + +`const cp = require('child_process'); cp.spawn(cmd, args, options)` is +verbatim what `cross-spawn` does, so every consumer of it inherited the bug: +`execa`, `which` (via `isexe`), and the MCP SDK's `StdioClientTransport`. Under +Perry, claude-code could *be* an MCP stdio server but never *connect* as one — +`claude mcp list` reported `✗ Failed to connect` for every configured stdio +server while `strace` showed no `execve` at all, because +`StdioClientTransport.start()` immediately dereferenced the `undefined` that +came back from `spawn()`. + +The router now falls through to the one canonical table instead of duplicating +it, and the normalization is split into `normalize_dispatch_module_name` so the +agreement between the two paths is unit-testable: every `.default` name in +`cjs_default_base_module` must normalize to the same base module the +property-read path picks, and must reach the same dispatch bucket. Adding a row +to the canonical table without teaching the router about it is now a test +failure rather than a silent `undefined`. diff --git a/crates/perry-runtime/src/object/native_module_dispatch.rs b/crates/perry-runtime/src/object/native_module_dispatch.rs index fa43e0aa64b..d921927c979 100644 --- a/crates/perry-runtime/src/object/native_module_dispatch.rs +++ b/crates/perry-runtime/src/object/native_module_dispatch.rs @@ -241,25 +241,16 @@ macro_rules! nm_general_closures { }; } -/// Thin router — extract+normalize the module name, dispatch via the per-module -/// registry. Names no bucket fn, so unused modules dead-strip. -pub(crate) unsafe fn dispatch_native_module_method( - obj: *const ObjectHeader, - method_name: &str, - args_ptr: *const f64, - args_len: usize, -) -> f64 { - // Extract the module name from field 0 of the namespace object - let module_field = js_object_get_field(obj as *mut _, 0); - let module_name = if module_field.is_string() { - let str_ptr = module_field.as_string_ptr(); - let len = (*str_ptr).byte_len as usize; - let data = (str_ptr as *const u8).add(std::mem::size_of::()); - std::str::from_utf8(std::slice::from_raw_parts(data, len)).unwrap_or("") - } else { - "" - }; - let (module_name, assert_skip_prototype) = match module_name { +/// Normalize the name stored on a native-module namespace object to the name the +/// per-module dispatch buckets are registered under, plus assert's +/// skip-prototype flag. +/// +/// Split out of `dispatch_native_module_method` so the `.default` coverage +/// is unit-testable against `cjs_default_base_module` — the same table the +/// property-READ path (`bound_native_callable_export_value`) normalizes +/// through. See `cjs_default_dispatch_names_resolve_to_a_bucket`. +pub(crate) fn normalize_dispatch_module_name(module_name: &str) -> (&str, bool) { + match module_name { "assert.instance" => ("assert", false), "assert.instance.skip" => ("assert", true), "assert/strict.instance" => ("assert/strict", false), @@ -291,8 +282,48 @@ pub(crate) unsafe fn dispatch_native_module_method( // has no arm — and returned `undefined`. The base `("punycode", …)` // arms below already implement decode/encode/toASCII/toUnicode. "punycode.default" => ("punycode", false), - _ => (module_name, false), + // #9485: every REMAINING `.default` CJS namespace normalizes + // through the one canonical table the property-read path already uses + // (`cjs_default_base_module`, shared with + // `bound_native_callable_export_value`). The hand-maintained arms above + // had drifted from it — `child_process.default`, `constants.default`, + // `dns.default`, `dns/promises.default`, `inspector.default`, + // `inspector/promises.default`, `module.default`, `repl.default`, + // `sea.default` and `wasi.default` were all missing — so a METHOD CALL + // on the namespace `const cp = require('child_process')` produces + // (`cp.spawn(...)`, `cp['spawn'](...)`) reached + // `nm_dispatch_lookup("child_process.default")`, found no bucket, and + // returned `undefined` WITHOUT spawning. The two-step forms + // (`const f = cp.spawn; f(...)`, `cp.spawn.call(...)`) always worked, + // because the property-read path normalizes through the canonical + // table — which is exactly the asymmetry that made cross-spawn (and + // therefore claude-code's MCP stdio client) fail silently. + other => match cjs_default_base_module(other) { + Some(base) => (base, false), + None => (other, false), + }, + } +} + +/// Thin router — extract+normalize the module name, dispatch via the per-module +/// registry. Names no bucket fn, so unused modules dead-strip. +pub(crate) unsafe fn dispatch_native_module_method( + obj: *const ObjectHeader, + method_name: &str, + args_ptr: *const f64, + args_len: usize, +) -> f64 { + // Extract the module name from field 0 of the namespace object + let module_field = js_object_get_field(obj as *mut _, 0); + let module_name = if module_field.is_string() { + let str_ptr = module_field.as_string_ptr(); + let len = (*str_ptr).byte_len as usize; + let data = (str_ptr as *const u8).add(std::mem::size_of::()); + std::str::from_utf8(std::slice::from_raw_parts(data, len)).unwrap_or("") + } else { + "" }; + let (module_name, assert_skip_prototype) = normalize_dispatch_module_name(module_name); let ctx = NmCtx { obj, args_ptr, @@ -335,3 +366,107 @@ pub(crate) use dispatch_q_u::{ }; pub(crate) use dispatch_util::nm_dispatch_util; pub(crate) use dispatch_v_z::{nm_dispatch_v8, nm_dispatch_vm, nm_dispatch_wasi, nm_dispatch_zlib}; + +#[cfg(test)] +mod cjs_default_dispatch_tests { + use super::*; + + /// Every `.default` namespace name `cjs_default_base_module` knows. + /// Kept spelled out so ADDING a row there without teaching the method-call + /// router about it is a test failure rather than a silent `undefined`. + const CJS_DEFAULT_NAMESPACES: &[&str] = &[ + "async_hooks.default", + "child_process.default", + "cluster.default", + "constants.default", + "dns.default", + "dns/promises.default", + "ffi.default", + "inspector.default", + "inspector/promises.default", + "module.default", + "node-pty.default", + "os.default", + "path.default", + "path.posix.default", + "path.win32.default", + "process.default", + "punycode.default", + "querystring.default", + "repl.default", + "sea.default", + "url.default", + "util.default", + "wasi.default", + ]; + + /// #9485: the method-CALL router must normalize a `.default` namespace + /// name exactly the way the property-READ path + /// (`bound_native_callable_export_value` → `cjs_default_base_module`) does. + /// The router used to carry its own hand-maintained list, and it had + /// drifted: `child_process.default` was missing, so + /// `require('child_process').spawn(...)` — cross-spawn's exact shape, and + /// therefore the MCP SDK's stdio client — dispatched under a name with no + /// bucket and returned `undefined` WITHOUT SPAWNING, while + /// `const f = cp.spawn; f(...)` worked. + #[test] + fn cjs_default_names_normalize_like_the_property_read_path() { + for name in CJS_DEFAULT_NAMESPACES { + let canonical = cjs_default_base_module(name).unwrap_or_else(|| { + panic!("{name} is not in cjs_default_base_module — update this list") + }); + let (dispatched, skip_prototype) = normalize_dispatch_module_name(name); + assert_eq!( + dispatched, canonical, + "method-call dispatch for `{name}` normalizes to `{dispatched}` but the \ + property-read path resolves it to `{canonical}`" + ); + assert!( + !skip_prototype, + "`{name}` is a CJS-default namespace, not an assert instance" + ); + } + } + + /// The consequence of the drift, stated directly: a `.default` name + /// must reach the SAME dispatch bucket its base module reaches. Modules + /// with no bucket at all (data-only namespaces such as `constants`) are + /// skipped — they have no methods to dispatch. + #[test] + fn cjs_default_names_reach_the_same_dispatch_bucket_as_the_base_module() { + use crate::object::native_module_registry::nm_dispatch_lookup; + let mut checked = 0usize; + for name in CJS_DEFAULT_NAMESPACES { + let base = cjs_default_base_module(name).expect("listed above"); + if nm_dispatch_lookup(base).is_none() { + continue; + } + let (dispatched, _) = normalize_dispatch_module_name(name); + assert!( + nm_dispatch_lookup(dispatched).is_some(), + "`{name}` normalizes to `{dispatched}`, which has no dispatch bucket — a \ + method call on that namespace would silently return undefined" + ); + checked += 1; + } + // Positive control: the loop must actually have asserted something. + assert!( + checked >= 15, + "expected most CJS-default namespaces to have a dispatch bucket, checked {checked}" + ); + } + + /// The exact regression, pinned by name. + #[test] + fn child_process_default_dispatches_as_child_process() { + assert_eq!( + normalize_dispatch_module_name("child_process.default"), + ("child_process", false) + ); + assert_eq!(normalize_dispatch_module_name("dns.default"), ("dns", false)); + assert_eq!( + normalize_dispatch_module_name("module.default"), + ("module", false) + ); + } +}