Skip to content

land #8880: restore the end/close arm on the stdin provider path - #8881

Merged
proggeramlug merged 2 commits into
mainfrom
land-8880-stdin-provider
Aug 27, 2026
Merged

land #8880: restore the end/close arm on the stdin provider path#8881
proggeramlug merged 2 commits into
mainfrom
land-8880-stdin-provider

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Lands #8880 — restores the end/close arm on the stdin listener provider path.

The fix is correct and needed. stdin_on_op / stdin_off_op are what the stdin object's native on / once / addListener delegate to — every registration that is not codegen's literal process.stdin.x(…) shape, i.e. 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, so those registrations hit _ => return and were silently discarded, and the end half of race(once("end"), timeout(3000)) could never win.

Verified on main before merging: stdin_off_op has only data / readable arms — no end/close. So the gap is real.

One correction to the PR description

It attributes the gap to work being "lost when #8861 was batch-landed". That is not what happened, and the record is worth keeping straight:

So this is completing #8861's third layer, not restoring something dropped. #8861 landed the two layers it actually contained — the STDIN_END_CALLBACKS GC rooting (which I added while landing, since the new list was missing from the root scanner) and STDIN_PULL_MODE.

Where I was incomplete: while landing #8864 I checked that the "end" | "close" arm survived at readline/mod.rs:1651 and reported the interaction as safe. That line is the syntactic extern's arm, not the provider's. Checking one of two paths and reporting on both is the gap worth recording.

Validation

  • all 30 lint-job gates pass
  • both new tests pass: provider_path_registers_end_listeners, provider_path_removes_end_listeners — and they assert the provider entry points directly, so this cannot silently regress again
  • perry-stdlib 124, perry-runtime 2716 — 0 failed

Summary by CodeRabbit

  • Bug Fixes

    • Fixed piped input commands that could hang when listening for stdin completion events.
    • Restored support for end and close events when stdin is accessed through aliases or passed as a parameter.
    • Ensured these listeners are properly removed, preventing stale callbacks from accumulating.
  • Tests

    • Added coverage for registering, triggering, and removing stdin completion listeners.

Ralph Küpper and others added 2 commits August 27, 2026 14:39
`echo hi | claude -p "…"` never completes on main. #8861 fixed this by
teaching three layers about `end` listeners; the batch landing kept the
GC rooting of STDIN_END_CALLBACKS and STDIN_PULL_MODE, but dropped the
`"end" | "close"` arm in stdin_on_op — the one that made it work.

stdin_on_op is the provider the stdin object's native on/once/addListener
delegate to: every registration that is NOT codegen's literal
process.stdin.x(…) shape, i.e. 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 those registrations hit
`_ => return` and were discarded, and the end half of its
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.

The two tests that guarded this were dropped with the arm, which is why CI
stayed green. Restored, asserting the provider entry points directly rather
than through js_readline_stdin_on — the extern path kept working, so it
cannot catch this. Sabotage-checked.

Bundle: 4/4 hang before, ~7s after. readline suite 21/21.
@proggeramlug
proggeramlug merged commit 245fe81 into main Aug 27, 2026
16 of 20 checks passed
@proggeramlug
proggeramlug deleted the land-8880-stdin-provider branch August 27, 2026 12:59
@coderabbitai

coderabbitai Bot commented Aug 27, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c6b4dac2-a32e-4696-8401-0e17f0f5f947

📥 Commits

Reviewing files that changed from the base of the PR and between ea39b9c and 2ed020c.

📒 Files selected for processing (3)
  • changelog.d/8879-stdin-end-provider-arm.md
  • crates/perry-stdlib/src/readline/mod.rs
  • crates/perry-stdlib/src/readline/mod_tests.rs

📝 Walkthrough

Walkthrough

The stdin provider now handles "end" and "close" listeners. Removal clears registered callbacks. Tests cover registration, EOF firing, and removal. The changelog records the fix and benchmark results.

Changes

Stdin provider listener lifecycle

Layer / File(s) Summary
Provider listener registration and removal
crates/perry-stdlib/src/readline/mod.rs, crates/perry-stdlib/src/readline/mod_tests.rs, changelog.d/8879-stdin-end-provider-arm.md
stdin_on_op stores "end" and "close" callbacks. stdin_off_op removes them and clears the matching close callback. Tests verify callback registration, EOF execution, and removal. The changelog documents the fix and benchmark results.

Estimated code review effort: 2 (Simple) | ~10 minutes

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch land-8880-stdin-provider

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant