fix(stdin): restore the end/close arm on the listener provider path (#8861 regression)

changelog.d/8879-stdin-end-provider-arm.md

@@ -0,0 +1,38 @@
+Restored the `end`/`close` arm on the stdin listener **provider** path, which
+was lost when #8861 was batch-landed. Without it `echo hi | claude -p "…"`
+never completes.
+
+#8861 fixed piped stdin for `-p` by teaching three layers about `end`
+listeners. Two of them landed: the GC rooting of `STDIN_END_CALLBACKS`, and
+`STDIN_PULL_MODE`. The third — the `"end" | "close"` arm in `stdin_on_op` —
+did not, and it is the one that made the fix work.
+
+`stdin_on_op` is the provider that the stdin object's native
+`on` / `once` / `addListener` methods delegate to: every registration that is
+**not** codegen's literal `process.stdin.x(…)` shape. That means an alias
+(`const s = process.stdin; s.once("end", …)`) or stdin passed as a parameter
+(`helper(process.stdin)`). Claude Code's print-mode reader is exactly the
+parameter form — `X71(process.stdin, 3000)`, then `stream.once("end", …)` on
+the parameter — so those registrations fell into `_ => return` and were
+silently discarded. The `end` half of the reader's
+`race(once("end"), timeout(3000))` could never win.
+
+`stdin_off_op` gets the mirror arm. #8864 removed the `end`/`close` clause from
+the removal path, so a provider-registered listener could be added but never
+removed — `removeListener` / `off` leaked it and the pump kept a stale callback
+pointer.
+
+The two tests that guarded this — `provider_path_registers_end_listeners` and
+`provider_path_removes_end_listeners` — were dropped alongside the arm, which is
+why CI stayed green through the regression. Both are restored here. They assert
+the provider entry points (`stdin_on_op` / `stdin_off_op`) **directly** rather
+than going through the `js_readline_stdin_on` extern, because the extern path
+kept working the whole time and therefore cannot catch this. Sabotage-checked:
+deleting the arm again fails `provider_path_registers_end_listeners`.
+
+Measured on the claude-code 2.1.112 bundle, `echo hello | cc -p "…"`:
+
+| build | result |
+|---|---|
+| before this fix | 4/4 hang (120 s timeout, no output) |
+| with the arm restored | completes in ~7 s |

crates/perry-stdlib/src/readline/mod.rs

@@ -376,6 +376,27 @@ extern "C" fn stdin_on_op(name_ptr: *const u8, name_len: usize, cb: i64, _once:
                 v.push(cb);
             }
         }
+        // #8861: `end`/`close` MUST be handled here, not only in the syntactic
+        // `js_readline_stdin_on` extern.
+        //
+        // This provider is what the stdin OBJECT's native `on`/`once`/
+        // `addListener` methods delegate to — i.e. every registration that does
+        // not match codegen's literal `process.stdin.x(…)` pattern: an alias
+        // (`const s = process.stdin; s.once("end", …)`) or stdin passed as a
+        // parameter (`helper(process.stdin)`). That is exactly what Claude
+        // Code's print-mode reader does: `X71(process.stdin, 3000)`, then
+        // `stream.once("end", …)` on the parameter inside.
+        //
+        // Falling into the `_ => return` below silently discards those
+        // listeners. Node fires the direct, aliased and parameter forms alike;
+        // without this arm perry fires only the direct one, so the `end` half
+        // of the reader's `race(once("end"), timeout(3000))` can never win and
+        // `echo hi | claude -p …` never completes.
+        "end" | "close" => {
+            if let Ok(mut v) = STDIN_END_CALLBACKS.lock() {
+                v.push(cb);
+            }
+        }
         _ => return,
     }
     try_register_pump();
@@ -410,6 +431,21 @@ extern "C" fn stdin_off_op(name_ptr: *const u8, name_len: usize, cb: i64) {
                 v.retain(|r| *r != cb);
             }
         }
+        // Mirror of the `end`/`close` arm in `stdin_on_op`. Without it a
+        // listener registered through the provider path can be added but never
+        // removed, so `removeListener`/`off` leaks it and the pump keeps a
+        // stale callback pointer.
+        "end" | "close" => {
+            if let Ok(mut v) = STDIN_END_CALLBACKS.lock() {
+                v.retain(|r| *r != cb);
+            }
+            CLOSE_CALLBACK.with(|slot| {
+                let mut slot = slot.borrow_mut();
+                if *slot == Some(cb) {
+                    *slot = None;
+                }
+            });
+        }
         _ => {}
     }
 }

crates/perry-stdlib/src/readline/mod_tests.rs

@@ -77,6 +77,55 @@ fn every_stdin_end_listener_fires() {
     );
 }
 
+/// The PROVIDER path — `stdin_on_op` / `stdin_off_op` — must handle
+/// `end`/`close` too, not just the syntactic `js_readline_stdin_on` extern
+/// that `every_stdin_end_listener_fires` above covers.
+///
+/// The provider is what the stdin object's native `on`/`once`/`addListener`
+/// delegate to, i.e. every registration that is NOT codegen's literal
+/// `process.stdin.x(…)` shape: an alias, or stdin passed as a parameter.
+/// Claude Code's print-mode reader is the parameter form — `X71(process.stdin,
+/// 3000)` then `stream.once("end", …)` inside — so without these arms its
+/// `race(once("end"), timeout(3000))` can never resolve via `end` and
+/// `echo hi | claude -p …` never completes.
+///
+/// These two tests guard arms that have now been lost twice, which is why they
+/// assert the provider entry points directly rather than going through the
+/// extern.
+#[test]
+fn provider_path_registers_end_listeners() {
+    let _g = reset();
+    let a = data_counter_callback();
+    let b = data_counter_callback();
+    stdin_on_op(b"end".as_ptr(), 3, a, 0);
+    stdin_on_op(b"close".as_ptr(), 5, b, 1);
+    assert_eq!(
+        STDIN_END_CALLBACKS.lock().map(|v| v.len()).unwrap_or(0),
+        2,
+        "aliased end/close registrations must reach the end-listener list"
+    );
+    EOF_REACHED.store(true, Ordering::Release);
+    js_readline_process_pending();
+    assert_eq!(
+        DATA_COUNT.with(|n| *n.borrow()),
+        2,
+        "listeners registered through the provider must fire at EOF"
+    );
+}
+
+#[test]
+fn provider_path_removes_end_listeners() {
+    let _g = reset();
+    let cb = data_counter_callback();
+    stdin_on_op(b"end".as_ptr(), 3, cb, 0);
+    stdin_off_op(b"end".as_ptr(), 3, cb);
+    assert_eq!(
+        STDIN_END_CALLBACKS.lock().map(|v| v.len()).unwrap_or(0),
+        0,
+        "removing an aliased end listener must clear it"
+    );
+}
+
 /// An `on("readable")` listener puts stdin in paused/pull mode, where the
 /// fd-0 reader must buffer bytes for `process.stdin.read()` instead of
 /// routing them to readline's line queue (which `read()` never drains).