fix(runtime): route Array.prototype mutators on Proxy receivers through their traps (unblocks the release gate)

changelog.d/6907-proxy-array-mutator-traps.md

@@ -0,0 +1 @@
+Fix `Array.prototype` mutators (`push`/`pop`/`shift`/`unshift`) on Proxy receivers reaching the dynamic dispatch path: the mutation was silently dropped (no trap fired, `undefined` returned) — immer's `draft.list.push(x)` shape. `array_proto_mutator` now routes Proxy receivers through spec-ordered `get`/`set`/`deleteProperty` trap sequences, byte-identical to Node.

crates/perry-runtime/src/array/generic.rs

@@ -1752,6 +1752,21 @@ pub fn dispatch_arraylike_read_method(
 /// engine; any other receiver yields `undefined`. `recv` is the call-site
 /// `this` (IMPLICIT_THIS) the thunk read.
 pub fn array_proto_mutator(recv: f64, method: &str, args_ptr: *const f64, args_len: usize) -> f64 {
+    // A Proxy receiver reaches this generic path whenever the eager HIR array
+    // fold bails (untyped receiver — #6397) and the dispatcher resolves the
+    // method through `Get(proxy, name)` → prototype thunk → here. Neither
+    // normalization below can represent it: `as_real_array` rightly rejects
+    // handle-band ids (deref = the #6279 segfault class) and
+    // `run_object_mutator` only accepts plain objects — so the mutation was
+    // silently DROPPED (immer's `draft.list.push(x)` returned undefined and
+    // mutated nothing). Route the traps instead.
+    if crate::proxy::js_proxy_is_proxy(recv) == 1 {
+        if let Some(r) = super::push_pop::proxy_array_mutator(recv, method, args_ptr, args_len) {
+            return r;
+        }
+        // Mutators not yet trap-routed (reverse/sort/splice/fill/copyWithin)
+        // keep the pre-existing fall-through.
+    }
     let arr = as_real_array(recv);
     if !arr.is_null() {
         return unsafe { real_array_mutator(arr, method, args_ptr, args_len) };

crates/perry-runtime/src/array/push_pop.rs

@@ -175,6 +175,163 @@ unsafe fn proxy_set_str_key(proxy: f64, key_bytes: &[u8], value: f64) {
     crate::proxy::js_proxy_set(proxy, key_f64, value);
 }
 
+/// `Get(proxy, <string key>)` through the proxy's `get` trap; fresh key
+/// string per call, same GC rationale as `proxy_set_str_key`.
+unsafe fn proxy_get_str_key(proxy: f64, key_bytes: &[u8]) -> f64 {
+    let key = crate::string::js_string_from_bytes(key_bytes.as_ptr(), key_bytes.len() as u32);
+    let key_f64 = crate::value::js_nanbox_string(key as i64);
+    crate::proxy::js_proxy_get(proxy, key_f64)
+}
+
+/// `HasProperty(proxy, <string key>)` through the proxy's `has` trap.
+unsafe fn proxy_has_str_key(proxy: f64, key_bytes: &[u8]) -> bool {
+    let key = crate::string::js_string_from_bytes(key_bytes.as_ptr(), key_bytes.len() as u32);
+    let key_f64 = crate::value::js_nanbox_string(key as i64);
+    crate::proxy::js_proxy_has(proxy, key_f64).to_bits() == crate::value::TAG_TRUE
+}
+
+/// Spec `DeletePropertyOrThrow(proxy, <string key>)`: routes the
+/// `deleteProperty` trap and throws the spec TypeError when the trap reports
+/// failure — `pop`/`shift`/`unshift` must abort BEFORE their length write
+/// rather than report a successful mutation.
+unsafe fn proxy_delete_str_key_or_throw(proxy: f64, key_bytes: &[u8]) {
+    let key = crate::string::js_string_from_bytes(key_bytes.as_ptr(), key_bytes.len() as u32);
+    let key_f64 = crate::value::js_nanbox_string(key as i64);
+    if crate::proxy::js_proxy_delete(proxy, key_f64).to_bits() != crate::value::TAG_TRUE {
+        let msg = format!(
+            "'deleteProperty' on proxy: trap returned falsish for property '{}'",
+            String::from_utf8_lossy(key_bytes)
+        );
+        crate::collection_iter::throw_type_error(&msg);
+    }
+}
+
+/// `Array.prototype` mutators on a Proxy receiver, spec-routed through the
+/// proxy's `get`/`set`/`deleteProperty` traps (§23.1.3.21 push, §23.1.3.19
+/// pop, §23.1.3.24 shift, §23.1.3.32 unshift — length reads/writes and every
+/// element move go through `[[Get]]`/`[[Set]]`, which is what fires the
+/// traps).
+///
+/// This is the receiver-normalization gap behind the `holder.list.push(3)`
+/// silent no-op: `array_proto_mutator` normalized the receiver with
+/// `as_real_array` (which rightly rejects handle-band proxy ids — deref'ing
+/// one as an ArrayHeader is the #6279 segfault class) and then
+/// `run_object_mutator` (proxy ids are not plain objects), so the whole call
+/// fell through to `undefined` WITHOUT mutating anything. The eager HIR fold
+/// used to hide this by calling `js_array_push_f64` (proxy-aware via
+/// `array_ptr_as_proxy`) directly, until #6397 correctly deferred untyped
+/// receivers to the runtime dispatch.
+///
+/// Returns `None` for mutators not yet routed (reverse/sort/splice/fill/
+/// copyWithin) — callers keep their previous fall-through behavior for those.
+pub(super) fn proxy_array_mutator(
+    proxy: f64,
+    method: &str,
+    args_ptr: *const f64,
+    args_len: usize,
+) -> Option<f64> {
+    let undefined = f64::from_bits(crate::value::TAG_UNDEFINED);
+    // Every trap below runs arbitrary user JS, which can allocate and move
+    // heap values. Root the proxy and the incoming args once and reload each
+    // carried value from its handle after any trap/allocating call (same
+    // pattern as `js_native_call_method`'s dispatch prologue). The proxy
+    // value itself is a registry handle id (not a heap pointer), but rooting
+    // it is free and keeps the discipline uniform.
+    let scope = crate::gc::RuntimeHandleScope::new();
+    let proxy_handle = scope.root_nanbox_f64(proxy);
+    let original_args: Vec<f64> = if args_len > 0 && !args_ptr.is_null() {
+        unsafe { std::slice::from_raw_parts(args_ptr, args_len).to_vec() }
+    } else {
+        Vec::new()
+    };
+    let arg_handles = scope.root_nanbox_f64_slice(&original_args);
+    let p = || proxy_handle.get_nanbox_f64();
+    let arg = |i: usize| -> f64 {
+        arg_handles
+            .get(i)
+            .map(|h| h.get_nanbox_f64())
+            .unwrap_or(undefined)
+    };
+    unsafe {
+        match method {
+            // §23.1.3.21 Array.prototype.push
+            "push" => {
+                let len = proxy_array_length(p());
+                for i in 0..args_len {
+                    proxy_set_str_key(p(), (len + i as u64).to_string().as_bytes(), arg(i));
+                }
+                let new_len = len + args_len as u64;
+                proxy_set_str_key(p(), b"length", new_len as f64);
+                Some(new_len as f64)
+            }
+            // §23.1.3.19 Array.prototype.pop
+            "pop" => {
+                let len = proxy_array_length(p());
+                if len == 0 {
+                    proxy_set_str_key(p(), b"length", 0.0);
+                    return Some(undefined);
+                }
+                let idx = (len - 1).to_string();
+                let value_handle = scope.root_nanbox_f64(proxy_get_str_key(p(), idx.as_bytes()));
+                proxy_delete_str_key_or_throw(p(), idx.as_bytes());
+                proxy_set_str_key(p(), b"length", (len - 1) as f64);
+                Some(value_handle.get_nanbox_f64())
+            }
+            // §23.1.3.24 Array.prototype.shift — HasProperty gates each move:
+            // a hole in the source deletes the destination instead of
+            // materializing an own `undefined`.
+            "shift" => {
+                let len = proxy_array_length(p());
+                if len == 0 {
+                    proxy_set_str_key(p(), b"length", 0.0);
+                    return Some(undefined);
+                }
+                let first_handle = scope.root_nanbox_f64(proxy_get_str_key(p(), b"0"));
+                for k in 1..len {
+                    let from = k.to_string();
+                    let to = (k - 1).to_string();
+                    if proxy_has_str_key(p(), from.as_bytes()) {
+                        let v_handle =
+                            scope.root_nanbox_f64(proxy_get_str_key(p(), from.as_bytes()));
+                        proxy_set_str_key(p(), to.as_bytes(), v_handle.get_nanbox_f64());
+                    } else {
+                        proxy_delete_str_key_or_throw(p(), to.as_bytes());
+                    }
+                }
+                proxy_delete_str_key_or_throw(p(), (len - 1).to_string().as_bytes());
+                proxy_set_str_key(p(), b"length", (len - 1) as f64);
+                Some(first_handle.get_nanbox_f64())
+            }
+            // §23.1.3.32 Array.prototype.unshift — same HasProperty gating on
+            // the right-shift loop.
+            "unshift" => {
+                let len = proxy_array_length(p());
+                let count = args_len as u64;
+                if count > 0 {
+                    for k in (0..len).rev() {
+                        let from = k.to_string();
+                        let to = (k + count).to_string();
+                        if proxy_has_str_key(p(), from.as_bytes()) {
+                            let v_handle =
+                                scope.root_nanbox_f64(proxy_get_str_key(p(), from.as_bytes()));
+                            proxy_set_str_key(p(), to.as_bytes(), v_handle.get_nanbox_f64());
+                        } else {
+                            proxy_delete_str_key_or_throw(p(), to.as_bytes());
+                        }
+                    }
+                    for i in 0..args_len {
+                        proxy_set_str_key(p(), i.to_string().as_bytes(), arg(i));
+                    }
+                }
+                let new_len = len + count;
+                proxy_set_str_key(p(), b"length", new_len as f64);
+                Some(new_len as f64)
+            }
+            _ => None,
+        }
+    }
+}
+
 /// Returns a pointer to the (possibly reallocated) array
 #[no_mangle]
 pub extern "C" fn js_array_push_f64(arr: *mut ArrayHeader, value: f64) -> *mut ArrayHeader {

crates/perry/tests/issue_5135_proxy_compound_and_function_tostring.rs

@@ -120,3 +120,97 @@ console.log(target.join(","), holder.list.length);
         "obj.list.push must mutate the proxied array through its set trap"
     );
 }
+
+/// The dispatch-tower variant of fix #3: when the receiver is untyped, #6397
+/// defers `proxy.push(x)` to `js_native_call_method`, whose proxy branch
+/// resolves the method via `Get(proxy, "push")` and lands in the
+/// `Array.prototype` thunk → `array_proto_mutator`. That normalization had no
+/// proxy branch (`as_real_array` rightly rejects handle-band ids;
+/// `run_object_mutator` only accepts plain objects), so the whole mutation was
+/// silently dropped — no trap fired, nothing mutated, `undefined` returned.
+/// Pin the spec trap sequence byte-for-byte against node: push fires
+/// `get(length)` then `set(<index>)` + `set(length)`, on top of the method
+/// lookup's own `get(push)`.
+#[test]
+fn proxy_array_push_via_dispatch_fires_traps_in_spec_order() {
+    let out = compile_and_run(
+        r#"
+let gets = 0, sets = 0;
+const target: any = [1, 2];
+const inner: any = new Proxy(target, {
+  get(t: any, k: any) { gets++; return t[k]; },
+  set(t: any, k: any, v: any) { sets++; t[k] = v; return true; },
+});
+const r0 = inner[0];
+inner[5] = 42;
+inner.push(9);
+console.log("r0=" + r0, "t5=" + target[5], "t=" + target.join(","), "gets=" + gets, "sets=" + sets);
+"#,
+    );
+    assert_eq!(
+        out, "r0=1 t5=42 t=1,2,,,,42,9 gets=3 sets=3\n",
+        "proxy push must fire get(length) + set(index) + set(length) traps and mutate the target"
+    );
+}
+
+/// Same dispatch path with a default (empty-handler) Proxy: every trap
+/// forwards to the target, so push must land on the target array.
+#[test]
+fn proxy_array_push_empty_handler_forwards_to_target() {
+    let out = compile_and_run(
+        r#"
+const target: any = [1, 2];
+const p: any = new Proxy(target, {});
+p.push(3);
+console.log(target.join(","), p.length);
+"#,
+    );
+    assert_eq!(
+        out, "1,2,3 3\n",
+        "empty-handler proxy push must forward to the target"
+    );
+}
+
+/// CodeRabbit follow-ups on the trap-routed mutators: holes must be preserved
+/// (`HasProperty` gates each element move; a source hole DELETES the
+/// destination instead of materializing an own `undefined`), and a
+/// `deleteProperty` trap returning false must throw the spec TypeError BEFORE
+/// the length write (DeletePropertyOrThrow). All expectations byte-identical
+/// to `node --experimental-strip-types`.
+#[test]
+fn proxy_array_mutators_preserve_holes_and_throw_on_refused_delete() {
+    let out = compile_and_run(
+        r#"
+{
+  const t: any = [1, , 3];
+  const p: any = new Proxy(t, {});
+  const r = p.shift();
+  console.log("shift:", r, t.join(","), t.length, (0 in t) ? "has0" : "hole0");
+}
+{
+  const t: any = [7, 8];
+  const p: any = new Proxy(t, {});
+  const r = p.pop();
+  console.log("pop:", r, t.join(","), t.length);
+}
+{
+  const t: any = [5, , 6];
+  const p: any = new Proxy(t, {});
+  const r = p.unshift(0);
+  console.log("unshift:", r, t.join(","), t.length, (2 in t) ? "has2" : "hole2");
+}
+{
+  const t: any = [1, 2];
+  const p: any = new Proxy(t, { deleteProperty() { return false; } });
+  try { p.pop(); console.log("delthrow: NO-THROW"); }
+  catch (e: any) { console.log("delthrow: threw", (e instanceof TypeError) ? "TypeError" : "other"); }
+  console.log("delthrow-after:", t.join(","), t.length);
+}
+"#,
+    );
+    assert_eq!(
+        out,
+        "shift: 1 ,3 2 hole0\npop: 8 7 1\nunshift: 4 0,5,,6 4 hole2\ndelthrow: threw TypeError\ndelthrow-after: 1,2 2\n",
+        "proxy mutators must preserve holes and DeletePropertyOrThrow"
+    );
+}