fix(hir): keep upgrade-callback wsId tagged ("ws","Client") across its own param binding

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

@@ -123,6 +123,7 @@ impl LoweringContext {
             native_instances_index: HashMap::new(),
             module_native_instances_index: HashMap::new(),
             func_return_native_instances_index: HashMap::new(),
+            prescan_protected_native_params: std::collections::HashMap::new(),
             native_modules_index: HashMap::new(),
             module_shadow_stack: Vec::new(),
             class_statics_index: HashMap::new(),
@@ -1194,12 +1195,16 @@ impl LoweringContext {
             .map(|(root, sub)| (root.as_str(), sub.as_str()))
     }
 
+    /// Register `local_name` as a native instance of `module_name`/`class_name`.
+    /// Returns `false` (a no-op) when the package is a `perry.compilePackages`
+    /// override (#5137) — callers that gate a side effect on successful
+    /// registration (e.g. `protect_native_param`) must check this.
     pub(crate) fn register_native_instance(
         &mut self,
         local_name: String,
         module_name: String,
         class_name: String,
-    ) {
+    ) -> bool {
         // #5137: if the user opted this package into `perry.compilePackages`,
         // its real npm source is being compiled and the binding resolves to
         // the compiled-from-source class. Registering a native instance here
@@ -1211,7 +1216,7 @@ impl LoweringContext {
         // is used. `is_native_module` already makes the same back-off for the
         // import-resolution side (#665).
         if is_compile_package_override(&module_name) {
-            return;
+            return false;
         }
         // Push the new index onto this name's shadow stack (innermost last).
         let idx = self.native_instances.len();
@@ -1221,6 +1226,7 @@ impl LoweringContext {
             .push(idx);
         self.native_instances
             .push((local_name, module_name, class_name));
+        true
     }
 
     /// Shadow any prior native-instance tag for `local_name` by pushing a
@@ -1248,11 +1254,38 @@ impl LoweringContext {
     /// saw `e.map`/`e.length`/`e.constructor` all read 0 while `e[0]` and
     /// `Array.prototype.map.call(e)` worked → `(number).join is not a function`).
     pub(crate) fn shadow_native_instance_if_present(&mut self, name: &str) {
+        // A pre-scan registered this exact param name as a native instance for
+        // the callback now being lowered (e.g. `wsId` for `server.on('upgrade',
+        // (req, wsId, head) => …)`). That tag is the FRESH, intended one — the
+        // param IS the native instance — so its own param binding must NOT
+        // tombstone it. One-shot: consume the protection so a later, unrelated
+        // param of the same name still shadows a genuinely stale tag.
+        // Consume only when this binding is at the depth the pre-scan anchored
+        // the protection to (the callback's own param scope). A same-named
+        // binding at any other depth must NOT consume it; `exit_scope` drops a
+        // never-consumed entry when the callback scope unwinds.
+        if self.prescan_protected_native_params.get(name).copied() == Some(self.scope_depth) {
+            self.prescan_protected_native_params.remove(name);
+            return;
+        }
         if self.lookup_native_instance(name).is_some() {
             self.shadow_native_instance(name.to_string());
         }
     }
 
+    /// Mark `name` as a pre-scan-designated native-instance param so the very
+    /// next `shadow_native_instance_if_present(name)` (the callback's own param
+    /// binding) skips tombstoning it. The pre-scan runs in the CALLER's scope,
+    /// one level above the callback, so anchor to `scope_depth + 1` (the
+    /// callback's param scope): the consume matches at exactly that depth, and a
+    /// never-consumed entry is dropped when the callback scope exits — it can't
+    /// linger into the caller scope and shadow a later, unrelated same-named
+    /// binding. See `prescan_protected_native_params`.
+    pub(crate) fn protect_native_param(&mut self, name: String) {
+        self.prescan_protected_native_params
+            .insert(name, self.scope_depth + 1);
+    }
+
     /// Truncate `native_instances` back to `mark`, keeping the
     /// `native_instances_index` shadow stacks in sync: every recorded index
     /// `>= mark` is popped (these belong to bindings whose scope is exiting),
@@ -1470,6 +1503,12 @@ impl LoweringContext {
     pub(crate) fn exit_scope(&mut self, mark: (usize, usize, usize)) {
         debug_assert!(self.scope_depth > 0, "exit_scope called at module depth");
         self.scope_depth = self.scope_depth.saturating_sub(1);
+        // Drop any pre-scan param protections registered in a scope deeper than
+        // the one we just returned to — their expected consumer (the callback's
+        // param binding) never fired, so they must not linger and skip a later
+        // same-named binding's tombstone.
+        self.prescan_protected_native_params
+            .retain(|_, depth| *depth <= self.scope_depth);
         self.scope_local_marks.pop();
         // #wall5: restore native-module shadowing for this scope.
         if let Some(m) = self.scope_module_shadow_marks.pop() {

crates/perry-hir/src/lower/expr_call/prescans.rs

@@ -79,7 +79,16 @@ pub(super) fn run_call_prescans(
     // perry-ext-http's `js_http_on` for client-side IncomingMessage
     // handles too via the cross-module on-listener path).
     if let Some(res_name) = pre_scan_node_http_client_callback_params(ctx, call) {
-        ctx.register_native_instance(res_name, "http".to_string(), "IncomingMessage".to_string());
+        // Protect only when registration actually happened — it no-ops under a
+        // `perry.compilePackages` override, and protecting without a fresh tag
+        // would let a later same-named param skip tombstoning a stale one.
+        if ctx.register_native_instance(
+            res_name.clone(),
+            "http".to_string(),
+            "IncomingMessage".to_string(),
+        ) {
+            ctx.protect_native_param(res_name);
+        }
     }
 
     // Issue #577 Phase 4 — `httpServer.on('upgrade', (req, wsId, head) => …)`
@@ -88,7 +97,16 @@ pub(super) fn run_call_prescans(
     // `wsId.close()` inside the handler dispatch via the Client-class
     // entries in NATIVE_MODULE_TABLE.
     if let Some(ws_id_name) = pre_scan_node_http_upgrade_params(ctx, call) {
-        ctx.register_native_instance(ws_id_name, "ws".to_string(), "Client".to_string());
+        // The arrow's own `wsId` param binding would otherwise tombstone this
+        // fresh tag via shadow_native_instance_if_present — protect it so
+        // `wsId.send`/`.on` inside the handler keep the Client-class dispatch.
+        // Protect only when registration actually happened (it no-ops under a
+        // compile-package override), else a later same-named param would skip
+        // tombstoning a genuinely stale tag.
+        if ctx.register_native_instance(ws_id_name.clone(), "ws".to_string(), "Client".to_string())
+        {
+            ctx.protect_native_param(ws_id_name);
+        }
     }
 
     // Issue #2211 — `request.on('socket', sock => …)` on a `ClientRequest`
@@ -98,7 +116,12 @@ pub(super) fn run_call_prescans(
     // handler dispatches through the class-filtered Socket rows in
     // NATIVE_MODULE_TABLE.
     if let Some(sock_name) = pre_scan_node_http_client_request_socket_params(ctx, call) {
-        ctx.register_native_instance(sock_name, "net".to_string(), "Socket".to_string());
+        // Protect only on successful registration (no-ops under a
+        // compile-package override) — see the `wsId` site above.
+        if ctx.register_native_instance(sock_name.clone(), "net".to_string(), "Socket".to_string())
+        {
+            ctx.protect_native_param(sock_name);
+        }
     }
 
     // perry/ui reactive Text: `Text(\`...${state.value}...\`)` where at least one

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

@@ -421,6 +421,24 @@ pub struct LoweringContext {
     /// truncated). The old lookup scanned FORWARD (first-match-wins), so the
     /// index keeps the FIRST pushed index per name (`entry().or_insert`).
     pub(crate) func_return_native_instances_index: HashMap<String, usize>,
+    /// Param names a pre-scan registered as native instances for the callback
+    /// it is ABOUT to lower (e.g. the `wsId` of `server.on('upgrade', (req,
+    /// wsId, head) => …)` tagged `("ws","Client")`). The callback's own param
+    /// binding then calls `shadow_native_instance_if_present(param)` to tombstone
+    /// stale leaked native tags — but here the tag is the FRESH, intended one,
+    /// so that shadow would wrongly erase it (the `wsId.send`/`.on` dead-channel
+    /// bug). One-shot per name: `shadow_native_instance_if_present` consumes the
+    /// entry and skips the tombstone exactly once.
+    ///
+    /// Each entry is anchored to the CALLBACK's param `scope_depth` (one below
+    /// the caller scope the pre-scan runs in); the consume matches at exactly
+    /// that depth and `exit_scope` drops entries deeper than the surviving depth.
+    /// So a protection whose expected consumer never fires (an unusual callback
+    /// shape, or a binding path that doesn't route through
+    /// `shadow_native_instance_if_present`) is cleared when the callback scope
+    /// exits and cannot leak into the caller scope to wrongly skip a later,
+    /// unrelated same-named binding's tombstone.
+    pub(crate) prescan_protected_native_params: std::collections::HashMap<String, usize>,
     /// Perf index for `native_modules` (push-only, never truncated). The old
     /// `lookup_native_module` scanned FORWARD (first-match-wins), so the index
     /// keeps the FIRST pushed index per name (`entry().or_insert`).

crates/perry-hir/tests/ws_upgrade_param_native_dispatch.rs

@@ -0,0 +1,71 @@
+// Regression: the `wsId` parameter of an HTTP `'upgrade'` handler
+// (`server.on('upgrade', (req, wsId, head) => …)`) must keep the
+// `("ws", "Client")` native-instance tag the upgrade pre-scan assigns it, so
+// `wsId.send(...)` / `wsId.on(...)` inside the handler lower to Client-class
+// `Expr::NativeMethodCall { module: "ws", … }`.
+//
+// The arrow's own `wsId` param binding calls `shadow_native_instance_if_present`
+// to tombstone any STALE native tag leaked from an outer scope — but here the
+// tag is the FRESH, intended one the pre-scan just registered. Without the
+// one-shot protection (`protect_native_param` / `prescan_protected_native_params`)
+// that binding wrongly erased it, and the method calls fell back to generic
+// dynamic dispatch (the post-upgrade "dead channel": `.send` / `.on` no longer
+// routed to the ws Client shims).
+//
+// The handler arrow lowers to an inline `Expr::Closure { body, .. }` nested as
+// the second argument of the `http.on` call (closures here are NOT lifted into
+// `module.functions`). perry-hir's public walker (`walk_expr_children`) skips
+// closure bodies and there is no public statement walker, so we assert over the
+// module's `Debug` rendering instead: only `Expr::NativeMethodCall` carries a
+// `module: "<name>"` field, so each `module: "ws"` occurrence is a surviving
+// Client-class dispatch. Without the fix the calls degrade to `Expr::Call` and
+// no `module: "ws"` appears.
+
+use perry_diagnostics::SourceCache;
+use perry_hir::{clear_current_module_source, fix_local_native_instances, lower_module};
+use perry_parser::parse_typescript_with_cache;
+
+fn lower(src: &str) -> perry_hir::Module {
+    let mut cache = SourceCache::new();
+    let parsed =
+        parse_typescript_with_cache(src, "/tmp/ws_upgrade_param.ts", &mut cache).expect("parse");
+    let mut module =
+        lower_module(&parsed.module, "test", "/tmp/ws_upgrade_param.ts").expect("lower");
+    clear_current_module_source();
+    fix_local_native_instances(&mut module);
+    module
+}
+
+#[test]
+fn upgrade_handler_wsid_keeps_client_class_dispatch_across_its_own_binding() {
+    let module = lower(
+        r#"
+        import { createServer } from "node:http";
+
+        const server = createServer((req: any, res: any) => {
+          res.end("ok");
+        });
+
+        server.on("upgrade", (req: any, wsId: any, _head: any) => {
+          wsId.send("perry-hello");
+          wsId.on("message", (_msg: any) => {});
+        });
+        "#,
+    );
+
+    let dump = format!("{module:#?}");
+    let ws_dispatches = dump.matches("module: \"ws\"").count();
+
+    // Both `wsId.send` and `wsId.on` must survive as ws Client NativeMethodCalls.
+    assert!(
+        ws_dispatches >= 2,
+        "expected wsId.send + wsId.on to lower as ws Client-class NativeMethodCalls, \
+         but found {ws_dispatches} `module: \"ws\"` dispatch(es). Lowered HIR:\n{dump}"
+    );
+    // `method: "send"` is unique to the ws Client call (the only other native
+    // call here is `http.on`), so its presence pins the discriminating case.
+    assert!(
+        dump.contains("method: \"send\""),
+        "wsId.send must dispatch as a ws NativeMethodCall. Lowered HIR:\n{dump}"
+    );
+}