Skip to content

node:repl: replace the stub with Node v26.3.0's REPL — v26 readline stack, acorn recoverable-parse + top-level await, completion, history, --interactive (82 vendored upstream tests) - #31827

Merged
dylan-conway merged 90 commits into
mainfrom
ciro/repl-node-tests
Jul 25, 2026

Conversation

@cirospaciari

@cirospaciari cirospaciari commented Jun 4, 2026

Copy link
Copy Markdown
Member

Brings node:repl from a stub that threw on use to a working port of Node's REPL, by porting the upstream implementation verbatim-where-possible and vendoring the upstream test suite.

82 of Node v26.3.0's 107 upstream REPL tests pass and are vendored here. The other 25 do not pass yet, so they are not committed — a vendored failing test needs an expectations.txt entry to keep CI green, which adds no coverage and hides the gap. They are enumerated under "Known gaps" below as follow-up work. This branch does not touch test/expectations.txt.

What's implemented

  • The full node:repl module ported from Node v26.3.0: repl.start(), REPLServer, Recoverable, REPL_MODE_SLOPPY/STRICT, builtinModules, the writer/eval option contracts, .break/.clear/.editor/.exit/.help/.save/.load commands, _/_error underscore assignment, and inspect.replDefaults passthrough.
  • The v26 internal/readline stack (interface, utils, callbacks, emitKeypressEvents, promises) replaces the older readline port, so node:readline and node:readline/promises are the same code Node ships; multiline editing, history navigation, reverse-i-search and keypress decoding come from upstream.
  • Recoverable-error detection and top-level await via acorn + acorn-walk vendored from Node's deps and evaluated with vm.Script (JSC builtin functions are non-constructors, which acorn's ES5 prototype style can't live under); const foo = { continues with | exactly like Node, and await input is rewritten through processTopLevelAwait.
  • Tab completion (completer, allowBlockingCompletions, member-expression evaluation with proxy/getter safety) and persistent history (setupHistory, NODE_REPL_HISTORY, dedup, size limits).
  • A --interactive CLI flag that boots the Node-compatible REPL (banner, NODE_REPL_* env handling, history file, repl.repl introspection). Bun's own -i is --install=fallback, so the long form is used for bun; under node emulation (argv0 node) -i means --interactive, as it does in Node.
  • -e under --interactive matches node -i -e: the script is evaluated after createInternalRepl (which starts the REPL synchronously), through the equivalent of Node's runScriptInContext — the CJS bindings are published onto the global and the body runs in global scope, so require/module/exports/__filename/__dirname resolve while var/function declarations still land on globalThis. An -e error stays fatal (exit 1, stdin never read).
  • Node's REPL error presentation: Uncaught <Error> formatting, REPL-frame trimming at the eval boundary, ERR_INVALID_REPL_INPUT-style decorated stack headers, ERR_* instanceof semantics, and cross-realm (vm-context) errors now render with name/message/stack through util.inspect (previously printed as ReferenceError {}).
  • internal/repl, internal/repl/await, internal/repl/history, internal/util/inspect are requirable in debug builds, matching what the upstream suite reaches via --expose-internals.

Native fixes the suite surfaced

  • vm: per-call displayErrors now gates the error-stack source-line decoration (Node only decorates when displayErrors !== false; we decorated unconditionally, so REPL syntax errors carried a spurious evalmachine.<anonymous>:1 prefix).
  • vm: new vm.Script(bad) now throws the SyntaxError at construction like Node, via an eager JSC::checkSyntax (Bun deferred it to run time). The REPL's recoverable-error flow depends on the eager throw. Noted as a double-parse to fold into compile-once in a follow-up.
  • vm: vm.compileFunction now gets the same arrow header as new vm.Script (Node's DecorateErrorStack runs for both). The header URL is resolved by the caller rather than inferred from an empty filename, because ScriptOptions seeds filename with an empty non-null string — so emptiness cannot distinguish "absent" from an explicit { filename: "" }. That also fixes new vm.Script(src, { filename: "" }), which rendered evalmachine.<anonymous>:1 where Node renders :1.
  • readline: Interface.prototype[Symbol.dispose].name is the string "[Symbol.dispose]", matching upstream's assignFunctionName; it was the raw Symbol, so util.inspect differed and any coercion of .name threw.
  • async_hooks: AsyncLocalStorage.run/snapshot called the callback with a spread, which routes through Array.prototype[Symbol.iterator]. Userland can delete that, and the REPL wraps every eval in replContext.run(), so deleting it made the REPL report Spread syntax requires ... instead of the user's own error. Node uses ReflectApply here for the same reason. (bind/exit have the same latent call-site spread and are left for a focused async_hooks change.)
  • inspect: errors from other realms (vm contexts) are recognized via isNativeError instead of instanceof Error.
  • builtins codegen: a regex literal at statement start silently truncated the rest of the bundled module — surfaced by internal/repl/await, worked around by hoisting; the codegen bug is a follow-up.

Vendored tests

82 upstream test files, byte-identical to Node v26.3.0 apart from two kinds of deviation, each carrying an inline bun: comment:

  • -i--interactive (8 files: test-repl-clear-immediate-crash, test-repl-harmony, test-repl-inspect-defaults, test-repl-require-after-write, test-repl-sigint, test-repl-sigint-nested-eval, test-repl-uncaught-exception-standalone, test-repl-unexpected-token-recoverable) — these spawn process.execPath, which is bun, not the node shim, so bun's -i (--install=fallback) applies and the long form is used instead.
  • test-readline-promises-tab-complete.js waits a macrotask per keystroke instead of a microtask; JSC settles the completion await chain across more microtask turns than V8. The assertions are unchanged.

test-readline-tab-complete.js had a Bun-weakened assertion (/^Tab completion error:[^]+error: message/, commented "modified to match bun's error message"); it is restored to the upstream text since the ported readline now produces Node's message.

Two upstream fixtures are force-added (test/js/node/test/fixtures/repl-load-multiline.js, repl-tab-completion-nested-repls.js): test/js/node/test/.gitignore excludes fixtures/repl* alongside the other upstream fixture trees this port never vendored, an exclusion that predates vendoring any REPL tests.

Known gaps (25 upstream tests not vendored)

Result previews need inspector-backed side-effect-free evaluation (Runtime.evaluate with throwOnSideEffect); JSC's inspector has no equivalent wired up, and sendInspectorCommand is a stub:
test-repl-custom-eval-previews · test-repl-history-navigation · test-repl-preview · test-repl-preview-newlines · test-repl-reverse-search · test-repl-strict-mode-previews · test-repl-mode · test-repl-inspector

JSC-vs-V8 stack frame format / error wording (Unexpected end of script vs Unexpected end of input, at readdirSync vs at Object.readdirSync, {} vs [3,2,1] in "is not iterable"), and message lengths that flip Node's own breakLength: 80 heuristic:
test-repl-pretty-stack · test-repl-pretty-custom-stack · test-repl-user-error-handler · test-repl-top-level-await · test-repl-unsafe-array-iteration · test-repl-underscore

SIGINT during eval does not interrupt vm execution yet:
test-repl-timeout-throw

Tab completion edge cases — test-repl-tab-complete needs Runtime.globalLexicalScopeNames (a V8 protocol method with no JSC counterpart) for let/const/class completion; test-repl-tab-complete-require and -import additionally assert that ./test-repl-tab-complete.js exists in the test directory, so they can only be vendored once it is:
test-repl-tab-complete · test-repl-tab-complete-unary-expressions · test-repl-tab-complete-require · test-repl-tab-complete-import

Assorted one-offs:
test-repl (large integration test; 250/265 cases pass, the remainder are V8-message-text assertions) · test-repl-require (module resolution edge cases) · test-repl-domain (node:domain integration incomplete) · test-repl-import-referrer (JSC renders module namespaces differently) · test-repl-unsupported-option (--input-type validation not implemented) · test-repl-uncaught-exception-async (uncaughtException listener guard timing)

Known limitations / follow-ups

  • vm.constants.DONT_CONTEXTIFY drops var persistence across script runs (lexical declarations survive, var doesn't), so the REPL context uses a contextified sandbox — Node's pre-v22 behavior — until the native context is fixed.
  • process.addUncaughtExceptionCaptureCallback / removeUncaughtExceptionCaptureCallback don't exist in Bun; shimmed over the single-callback set/clear API inside the REPL only.
  • Stacks materialize eagerly under JSC, so Node's overrideStackTrace-based REPL frame trimming is reproduced in decorateErrorStack instead of at capture time.
  • process.config.variables.v8_enable_i8n_support is misspelled (Node reads v8_enable_i18n_support), so common.hasIntl is always false and the suite silently skips its Intl-gated assertions. Out of scope here — it affects ~45 vendored tests across the tree and deserves its own change.

Fixes #28478


no test proof · iteration 16 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/cli/run/run-eval.test.ts test/js/bun/repl/repl.test.ts test/js/node/util/node-inspect-tests/parallel/util-inspect.test.js

@robobun

robobun commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator
Updated 7:50 PM PT - Jul 24th, 2026

@dylan-conway, your commit 0a2d409 is building: #80045

@coderabbitai

coderabbitai Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds Node-compatible REPL entrypoint, ports Node readline internals (keypress parsing, Interface, utils, promises, callbacks), implements node:repl, CLI --interactive wiring, VM displayErrors gating, and extensive Node-vendored REPL/readline tests.

Changes

Node REPL, readline, and CLI wiring

Readline primitives and CSI helpers / File(s): src/js/internal/readline/utils.js, src/js/internal/readline/callbacks.js, src/js/internal/readline/emitKeypressEvents.js, src/js/internal/readline/promises.js
Adds CSI template helper, UTF-16-aware string helpers, key/escape parsing generator, emitKeypressEvents wiring, cursor/clear callbacks, and a promises-based Readline class for batched CSI writes.

Readline Interface and internal REPL subsystems / File(s): src/js/internal/readline/interface.js, src/js/internal/repl/*, src/js/internal/repl/completion.js, src/js/internal/repl/history.js, src/js/internal/repl/await.js, src/js/internal/repl/node-*
Implements Interface editing engine (TTY/non-TTY), completion (filesystem, import/require, guarded eval), persistent ReplHistory, top-level await preprocessing, node-inspect/primordials/node-shims/error shims for REPL isolation and VM integration.

Public node:readline and node:repl ports / File(s): src/js/node/readline.js, src/js/node/repl.js, src/js/eval/node-repl.ts
Exports Node-compatible readline API surfaces (Interface, promises, helpers) and a full node:repl implementation with start/REPLServer/writer/Recoverable; adds standalone bun --interactive entrypoint that embeds node-repl.ts and manages history flush on exit.

Runtime, CLI, and VM wiring / File(s): src/runtime/cli/*, src/options_types/context.rs, src/jsc/bindings/NodeVMScript.cpp, src/resolve_builtins/HardcodedModule.rs
Adds --interactive flag and early dispatch to exec_node_repl, exposes internal Node modules for tests, and gates NodeVMScript exception decoration by displayErrors.

Tests and expectations / File(s): test/js/node/test/..., test/expectations.txt
Adds many Node-vendored REPL/readline tests covering completion, history, preview, editor mode, signals, top-level await, and updates expectations for the vendored suite.

  • Suggested reviewers:
    • Jarred-Sumner
    • alii
🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description covers the PR purpose, but it does not follow the required template and omits a clear "How did you verify your code works?" section. Rewrite it using the required headings "### What does this PR do?" and "### How did you verify your code works?", and add concrete verification steps.
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR fully addresses issue #28478 by implementing repl.start() and the complete node:repl module, enabling the exact use case reported (import repl from 'node:repl'; repl.start('$ ')) to work as expected.
Out of Scope Changes check ✅ Passed All changes are within scope: implementing node:repl and its dependencies, internal readline stack, testing infrastructure, and supporting native fixes. No unrelated refactoring or feature creep detected.
Title check ✅ Passed The title clearly summarizes the main change: replacing the REPL stub with a full Node v26.3.0-compatible implementation and tests.

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

@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

This PR may be a duplicate of:

  1. Implement node:repl start(), REPLServer, and Recoverable #28480 - Also implements node:repl (REPLServer, Recoverable, repl.start()) and fixes the same issue repl.start not implemented in Bun #28478

🤖 Generated with Claude Code

Comment thread src/runtime/cli/run_command.rs Outdated
Comment thread src/js/internal/repl/node-errors.js Outdated
Comment thread src/js/internal/repl/history.js Outdated
Comment thread src/js/node/repl.js
Comment thread src/js/node/repl.js Outdated
Comment thread src/js/internal/repl/node-shims.js
Comment thread src/js/internal/repl/node-shims.js

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/js/internal/repl/await.js`:
- Around line 71-86: registerVariableDeclarationIdentifiers currently assumes
every pattern node is non-null and one of Identifier/ObjectPattern/ArrayPattern;
this breaks on elisions, rest elements and assignment defaults. Update
registerVariableDeclarationIdentifiers to: return early on null nodes (handle
array elisions), add a case for "RestElement" that recurses into node.argument,
add a case for "AssignmentPattern" that recurses into node.left, and when
iterating ObjectPattern.properties handle Property and RestElement by recursing
into property.value || property.argument and letting the above cases handle
nested patterns so bindings are correctly pushed into
variableIdentifiersToHoist[variableKind === "var" ? 0 : 1][1].

In `@src/js/internal/repl/completion.js`:
- Around line 282-286: The REPL completion has two issues: replace incorrect
StringPrototypeIncludes usage with ArrayPrototypeIncludes for the extensions
membership checks (search for StringPrototypeIncludes(extensions, extension)
near the completion code paths that push into group and the other occurrence
~343) so the array is checked correctly, and change propHasGetterOrIsProxy to
walk the prototype chain when calling ObjectGetOwnPropertyDescriptor so you
never read obj[prop] (which can trigger inherited getters) before deciding
isProxy; use descriptor lookup on each prototype and only call isProxy on the
property value if the descriptor found is a data descriptor on that exact owner
rather than invoking getters.
- Around line 809-818: propHasGetterOrIsProxy currently only inspects
ObjectGetOwnPropertyDescriptor(obj, prop) and falls back to reading obj[prop]
(via isProxy(obj[prop])), which can invoke inherited getters; change it to walk
the prototype chain without reading the property: iterate using
ObjectGetPrototypeOf starting from obj and call ObjectGetOwnPropertyDescriptor
on each prototype until a descriptor is found; if any descriptor has a getter
(descriptor.get is a function) return cb(true); only after the prototype walk,
if no descriptor exists, you may safely call isProxy but avoid any intermediate
property reads—use these symbols: propHasGetterOrIsProxy,
ObjectGetOwnPropertyDescriptor, ObjectGetPrototypeOf, isProxy, and cb.

In `@src/js/internal/repl/history.js`:
- Around line 399-407: The removal in kOnExit fails because it calls
this[kContext].off("line", this[kOnLine].bind(this)) which is a new function;
capture and reuse a single bound listener instead: create and store a property
(e.g. this[kOnLineBound] = this[kOnLine].bind(this]) when the REPL initializes
or when the listener is first added, use this[kOnLineBound] when calling
this[kContext].on("line", ...) and when removing with this[kContext].off("line",
this[kOnLineBound]), and also use the stored bound reference when re-attaching
in the kIsFlushing branch to ensure .off/.once operate on the same function
object.

In `@src/js/internal/repl/node-shims.js`:
- Around line 71-73: The stubbed sendInspectorCommand currently always calls
onError(), which prevents getGlobalLexicalScopeNames (in completion.js) from
receiving a success response and breaks lexical-scope completion; update
sendInspectorCommand to treat onError as a true error path and instead invoke
the success callback cb with an appropriate empty/success response (or a
simulated inspector response that allows getGlobalLexicalScopeNames to proceed)
and only call onError when a real error occurs, referencing the
sendInspectorCommand function and getGlobalLexicalScopeNames to locate where the
behavior must change.

In `@src/js/node/repl.js`:
- Around line 1261-1271: countMatches returns a number but the code treats its
results as arrays (dw.length / up.length) causing NaN; change the depth
calculation to use the numeric counts (e.g., let depth = dw - up) and ensure you
reference the countMatches return values (dw and up) directly when updating
self.lines.level or any depth logic in the REPL; keep RegExpPrototypeExec usage
as-is but compute depth from the numeric dw and up results.

In `@src/resolve_builtins/HardcodedModule.rs`:
- Around line 179-186: The HardcodedModule enum variants NodeInternalRepl,
NodeInternalReplAwait, NodeInternalReplHistory, and NodeInternalUtilInspect
currently serialize as "internal:..." but your resolver keys use "internal/...";
update the #[strum(serialize = "...")] attributes for these variants to use the
slash form (e.g. "internal/repl", "internal/repl/await",
"internal/repl/history", "internal/util/inspect") so serialized specifiers match
the resolver keys used elsewhere.

In `@src/runtime/cli/mod.rs`:
- Around line 1485-1490: The early-return fast-path that calls
RunCommand::exec_node_repl when Tag::AutoCommand &&
ctx.runtime_options.interactive && ctx.runtime_options.eval.script.is_empty()
can skip executing provided entrypoints; update the condition to also require
that no positional args are present (e.g., add && ctx.positionals.is_empty()) so
the REPL is only entered when there is neither a script nor any positional
entrypoint, preserving the later logic that calls RunCommand::exec_with_cfg for
provided scripts/positionals.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 80af3ae2-699b-40d4-8f74-a4f478aade98

📥 Commits

Reviewing files that changed from the base of the PR and between 977f054 and 9780288.

📒 Files selected for processing (136)
  • oxlint.json
  • src/js/eval/node-repl.ts
  • src/js/internal/readline/callbacks.js
  • src/js/internal/readline/emitKeypressEvents.js
  • src/js/internal/readline/interface.js
  • src/js/internal/readline/promises.js
  • src/js/internal/readline/utils.js
  • src/js/internal/repl.js
  • src/js/internal/repl/acorn-walk.js
  • src/js/internal/repl/acorn.js
  • src/js/internal/repl/await.js
  • src/js/internal/repl/completion.js
  • src/js/internal/repl/history.js
  • src/js/internal/repl/node-errors.js
  • src/js/internal/repl/node-inspect.js
  • src/js/internal/repl/node-primordials.js
  • src/js/internal/repl/node-shims.js
  • src/js/internal/repl/utils.js
  • src/js/internal/util/inspect.js
  • src/js/node/readline.js
  • src/js/node/readline.promises.js
  • src/js/node/readline.promises.ts
  • src/js/node/readline.ts
  • src/js/node/repl.js
  • src/js/node/repl.ts
  • src/jsc/bindings/NodeVMScript.cpp
  • src/options_types/context.rs
  • src/resolve_builtins/HardcodedModule.rs
  • src/runtime/cli/Arguments.rs
  • src/runtime/cli/mod.rs
  • src/runtime/cli/run_command.rs
  • src/runtime/jsc_hooks.rs
  • test/js/node/test/common/repl.js
  • test/js/node/test/parallel/test-repl-array-prototype-tempering.js
  • test/js/node/test/parallel/test-repl-async-iife.js
  • test/js/node/test/parallel/test-repl-autocomplete.js
  • test/js/node/test/parallel/test-repl-autolibs.js
  • test/js/node/test/parallel/test-repl-clear-immediate-crash.js
  • test/js/node/test/parallel/test-repl-cli-eval.js
  • test/js/node/test/parallel/test-repl-colors.js
  • test/js/node/test/parallel/test-repl-completion-on-getters-disabled.js
  • test/js/node/test/parallel/test-repl-context.js
  • test/js/node/test/parallel/test-repl-custom-eval-previews.js
  • test/js/node/test/parallel/test-repl-custom-eval.js
  • test/js/node/test/parallel/test-repl-definecommand.js
  • test/js/node/test/parallel/test-repl-domain.js
  • test/js/node/test/parallel/test-repl-editor.js
  • test/js/node/test/parallel/test-repl-empty.js
  • test/js/node/test/parallel/test-repl-end-emits-exit.js
  • test/js/node/test/parallel/test-repl-envvars.js
  • test/js/node/test/parallel/test-repl-eval-error-after-close.js
  • test/js/node/test/parallel/test-repl-function-definition-edge-case.js
  • test/js/node/test/parallel/test-repl-harmony.js
  • test/js/node/test/parallel/test-repl-history-dedup-multiline.js
  • test/js/node/test/parallel/test-repl-history-init-fail-leak.js
  • test/js/node/test/parallel/test-repl-history-navigation.js
  • test/js/node/test/parallel/test-repl-history-perm.js
  • test/js/node/test/parallel/test-repl-import-referrer.js
  • test/js/node/test/parallel/test-repl-inspect-defaults.js
  • test/js/node/test/parallel/test-repl-inspector.js
  • test/js/node/test/parallel/test-repl-let-process.js
  • test/js/node/test/parallel/test-repl-load-multiline-from-history.js
  • test/js/node/test/parallel/test-repl-load-multiline-no-trailing-newline.js
  • test/js/node/test/parallel/test-repl-load-multiline.js
  • test/js/node/test/parallel/test-repl-mode.js
  • test/js/node/test/parallel/test-repl-multiline-navigation-while-adding.js
  • test/js/node/test/parallel/test-repl-multiline-navigation.js
  • test/js/node/test/parallel/test-repl-multiline.js
  • test/js/node/test/parallel/test-repl-multiple-instances-async-error.js
  • test/js/node/test/parallel/test-repl-no-terminal-restore-process-listeners.js
  • test/js/node/test/parallel/test-repl-no-terminal.js
  • test/js/node/test/parallel/test-repl-null-thrown.js
  • test/js/node/test/parallel/test-repl-null.js
  • test/js/node/test/parallel/test-repl-options.js
  • test/js/node/test/parallel/test-repl-permission-model.js
  • test/js/node/test/parallel/test-repl-persistent-history.js
  • test/js/node/test/parallel/test-repl-preprocess-top-level-await.js
  • test/js/node/test/parallel/test-repl-pretty-custom-stack.js
  • test/js/node/test/parallel/test-repl-pretty-stack-custom-writer.js
  • test/js/node/test/parallel/test-repl-pretty-stack.js
  • test/js/node/test/parallel/test-repl-preview-newlines.js
  • test/js/node/test/parallel/test-repl-preview-timeout.js
  • test/js/node/test/parallel/test-repl-preview.js
  • test/js/node/test/parallel/test-repl-programmatic-history-setup-history.js
  • test/js/node/test/parallel/test-repl-programmatic-history.js
  • test/js/node/test/parallel/test-repl-recoverable.js
  • test/js/node/test/parallel/test-repl-require-after-write.js
  • test/js/node/test/parallel/test-repl-require-cache.js
  • test/js/node/test/parallel/test-repl-require-context.js
  • test/js/node/test/parallel/test-repl-require-self-referential.js
  • test/js/node/test/parallel/test-repl-require.js
  • test/js/node/test/parallel/test-repl-reset-event.js
  • test/js/node/test/parallel/test-repl-reverse-search.js
  • test/js/node/test/parallel/test-repl-save-load-editor-mode.js
  • test/js/node/test/parallel/test-repl-save-load-invalid-save.js
  • test/js/node/test/parallel/test-repl-save-load-load-dir.js
  • test/js/node/test/parallel/test-repl-save-load-load-non-existent.js
  • test/js/node/test/parallel/test-repl-save-load-load-without-name.js
  • test/js/node/test/parallel/test-repl-save-load-save-without-name.js
  • test/js/node/test/parallel/test-repl-save-load.js
  • test/js/node/test/parallel/test-repl-setprompt.js
  • test/js/node/test/parallel/test-repl-sigint-nested-eval.js
  • test/js/node/test/parallel/test-repl-sigint.js
  • test/js/node/test/parallel/test-repl-stdin-push-null.js
  • test/js/node/test/parallel/test-repl-strict-mode-previews.js
  • test/js/node/test/parallel/test-repl-syntax-error-stack.js
  • test/js/node/test/parallel/test-repl-tab-complete-buffer.js
  • test/js/node/test/parallel/test-repl-tab-complete-computed-props.js
  • test/js/node/test/parallel/test-repl-tab-complete-crash.js
  • test/js/node/test/parallel/test-repl-tab-complete-custom-completer.js
  • test/js/node/test/parallel/test-repl-tab-complete-files.js
  • test/js/node/test/parallel/test-repl-tab-complete-import.js
  • test/js/node/test/parallel/test-repl-tab-complete-nested-repls.js
  • test/js/node/test/parallel/test-repl-tab-complete-new-expression.js
  • test/js/node/test/parallel/test-repl-tab-complete-no-warn.js
  • test/js/node/test/parallel/test-repl-tab-complete-nosideeffects.js
  • test/js/node/test/parallel/test-repl-tab-complete-on-editor-mode.js
  • test/js/node/test/parallel/test-repl-tab-complete-require.js
  • test/js/node/test/parallel/test-repl-tab-complete-unary-expressions.js
  • test/js/node/test/parallel/test-repl-tab-complete.js
  • test/js/node/test/parallel/test-repl-tab.js
  • test/js/node/test/parallel/test-repl-throw-null-or-undefined.js
  • test/js/node/test/parallel/test-repl-top-level-await.js
  • test/js/node/test/parallel/test-repl-uncaught-exception-after-input-ended.js
  • test/js/node/test/parallel/test-repl-uncaught-exception-async.js
  • test/js/node/test/parallel/test-repl-uncaught-exception-evalcallback.js
  • test/js/node/test/parallel/test-repl-uncaught-exception-standalone.js
  • test/js/node/test/parallel/test-repl-uncaught-exception.js
  • test/js/node/test/parallel/test-repl-underscore.js
  • test/js/node/test/parallel/test-repl-unexpected-token-recoverable.js
  • test/js/node/test/parallel/test-repl-unsafe-array-iteration.js
  • test/js/node/test/parallel/test-repl-unsupported-option.js
  • test/js/node/test/parallel/test-repl-use-global.js
  • test/js/node/test/parallel/test-repl-user-error-handler.js
  • test/js/node/test/parallel/test-repl.js
  • test/js/node/test/sequential/test-repl-timeout-throw.js
💤 Files with no reviewable changes (3)
  • src/js/node/readline.promises.ts
  • src/js/node/repl.ts
  • src/js/node/readline.ts

Comment thread src/js/internal/repl/await.js
Comment thread src/js/internal/repl/completion.js
Comment thread src/js/internal/repl/completion.js
Comment thread src/js/internal/repl/history.js
Comment thread src/js/internal/repl/node-shims.js
Comment thread src/js/node/repl.js
Comment thread src/js/node/repl.js
Comment thread src/resolve_builtins/HardcodedModule.rs
Comment thread src/runtime/cli/mod.rs Outdated
Comment thread src/js/node/readline.js
Comment thread src/runtime/cli/mod.rs Outdated
Comment thread src/js/eval/node-repl.ts Outdated
Comment thread src/js/eval/node-repl.ts Outdated
Comment thread src/js/internal/repl/node-shims.js

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/js/node/readline.js`:
- Around line 515-522: The current plain assignment to
__node_module__.exports[Symbol.for("__BUN_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED__")]
makes the Bun-only hook enumerable and discoverable; change it to use
Object.defineProperty on __node_module__.exports with the same
Symbol.for("__BUN_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED__") key and set
enumerable: false (and appropriate configurable/writable flags as needed) so the
CSI/utils hook is non-enumerable and not leaked via reflection.

In `@src/js/node/repl.js`:
- Around line 1477-1479: The current plain assignment
__node_module__.exports[Symbol.for("bun.repl.kStandaloneREPL")] =
kStandaloneREPL creates a writable, enumerable own property on the public
node:repl export; instead define the symbol property with Object.defineProperty
on __node_module__.exports using the same Symbol.for("bun.repl.kStandaloneREPL")
and value kStandaloneREPL but mark it non-enumerable (enumerable: false) and
non-writable (writable: false) so it does not appear on the public export
surface (you may keep configurable: true if needed).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 0eefd4a6-c2af-4b8f-b274-b5e47302be35

📥 Commits

Reviewing files that changed from the base of the PR and between 9780288 and 00311fc.

📒 Files selected for processing (10)
  • scripts/build/flags.ts
  • src/js/eval/node-repl.ts
  • src/js/internal/repl/node-shims.js
  • src/js/internal/util/inspect.js
  • src/js/node/readline.js
  • src/js/node/repl.js
  • src/runtime/cli/mod.rs
  • test/expectations.txt
  • test/js/node/test/parallel/test-readline-promises-tab-complete.js
  • test/js/node/test/parallel/test-readline-tab-complete.js

Comment thread src/js/node/readline.js Outdated
Comment thread src/js/node/repl.js Outdated
Comment thread test/expectations.txt Outdated
Comment thread src/js/internal/repl/node-errors.js Outdated
@cirospaciari
cirospaciari force-pushed the ciro/repl-node-tests branch from 48067f1 to 9129e14 Compare June 5, 2026 02:02
@cirospaciari

Copy link
Copy Markdown
Member Author

The x64-asan lane was aborting every test that loads node:repl with Error parsing builtin: Unrecognized token 'eval'. Root cause: assertion-enabled JSC rejects builtin sources containing an eval member access, and --minify-syntax folds the carefully written x["eval"] accesses back into x.eval. Reproduced with the CI asan artifact (BUN_JSC_exposePrivateIdentifiers=1 parses arbitrary files in builtin mode, which makes a handy oracle) and fixed by switching the five access sites to destructuring reads, ReflectApply calls, and one ObjectDefineProperty write — forms the minifier leaves alone. The regenerated minified bundles now contain no eval member tokens. Also fixed test-repl-colors.js (runner exports FORCE_COLOR=0; same delete-the-env workaround as test-console-tty-colors.js) and rebased on main.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/js/internal/readline/interface.js`:
- Around line 158-197: The code reassigns input = input.input before calling
this.setupHistoryManager(input), which drops the original options object (losing
onHistoryFileLoaded) so ReplHistory.initialize never gets the callback; fix by
passing the original options object to setupHistoryManager (i.e., call
this.setupHistoryManager(originalOptions) or retain a variable like optionsObj
for the outer object) while still extracting and assigning input.input to the
internal input stream variable; ensure setupHistoryManager and
ReplHistory.initialize receive the full options object including
onHistoryFileLoaded.

In `@src/js/internal/repl/await.js`:
- Around line 12-219: The module directly calls mutable built-ins
(Array.prototype.includes/forEach, Object.keys,
String.prototype.startsWith/includes/split/repeat, RegExp/Symbol.replace) which
can be monkey-patched; update processTopLevelAwait, visitorsWithoutAncestors and
the visitors construction to use Bun’s primordial/tamper-resistant intrinsics
(the $ prefixed primordials used across src/js/**) instead: replace calls like
["ForOfStatement","ForInStatement"].includes, node.declarations.forEach,
variableIdentifiersToHoist.forEach, Object.keys(walk.base),
e.message.startsWith, src.split,
String.prototype.startsWith/includes/split/repeat and
kParenMessageRe[Symbol.replace] with the corresponding
$ArrayIncludes/$ArrayForEach/$ObjectKeys/$StringStartsWith/$StringIncludes/$StringSplit/$StringRepeat/$SymbolReplace
(invoked with .call/.apply when necessary) so the AST walk, replacement logic in
processTopLevelAwait, and hoisting (variableIdentifiersToHoist,
registerVariableDeclarationIdentifiers, visitors) are resistant to prototype
tampering.

In `@src/js/internal/repl/node-primordials.js`:
- Around line 12-102: The primordials shim currently invokes prototype methods
at call time (e.g., ArrayFrom, ArrayPrototypePush/PushApply,
PromisePrototypeThen, RegExpPrototypeExec, StringPrototypeTrim,
ArrayPrototypeSlice, etc.), leaving behavior vulnerable to prototype tampering;
update the module to capture the original intrinsic functions once at module
initialization and call them via their safe helpers (use .$call for single-call
receivers and .$apply for variadic/apply usage) so Array.from,
Array.prototype.push, Promise.prototype.then, RegExp.prototype.exec,
String.prototype.trim, Array.prototype.slice, String.prototype.replaceAll, etc.
are invoked via the captured intrinsics rather than by calling the
possibly-mutated runtime properties (adjust ArrayFrom, ArrayPrototypePush,
ArrayPrototypePushApply, PromisePrototypeThen, RegExpPrototypeExec,
StringPrototypeTrim and other listed exports to use the captured originals).

In `@src/js/node/repl.js`:
- Around line 1497-1500: Replace the global Object.defineProperty call with the
primordial-safe ObjectDefineProperty when defining the standalone REPL export;
locate the call that targets __node_module__.exports and
Symbol.for("bun.repl.kStandaloneREPL") and invoke
ObjectDefineProperty(__node_module__.exports,
Symbol.for("bun.repl.kStandaloneREPL"), { __proto__: null, value:
kStandaloneREPL }) instead, preserving the same descriptor fields so the export
hook remains identical but uses the safe primordial helper.

In `@test/js/node/test/parallel/test-repl-persistent-history.js`:
- Around line 186-187: The test currently starts runTest() on the readable's
'unpipe' event which can fire before the writable has flushed; change the copy
to wait for the writable to finish instead — i.e., use the writable stream
returned by fs.createWriteStream(historyPath) and start runTest() from its
'finish' event (or use stream.pipeline to copy and call runTest() in the
pipeline's callback). Update the code that constructs the streams
(fs.createReadStream and fs.createWriteStream) and replace the '.on("unpipe", ()
=> runTest())' usage so runTest() is invoked only after the write stream's
'finish' (or pipeline completion).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 592b12a1-68bb-49b5-8f6d-2c087f2eb3bd

📥 Commits

Reviewing files that changed from the base of the PR and between 00311fc and 9129e14.

📒 Files selected for processing (141)
  • oxlint.json
  • scripts/build/flags.ts
  • src/js/eval/node-repl.ts
  • src/js/internal/readline/callbacks.js
  • src/js/internal/readline/emitKeypressEvents.js
  • src/js/internal/readline/interface.js
  • src/js/internal/readline/promises.js
  • src/js/internal/readline/utils.js
  • src/js/internal/repl.js
  • src/js/internal/repl/acorn-walk.js
  • src/js/internal/repl/acorn.js
  • src/js/internal/repl/await.js
  • src/js/internal/repl/completion.js
  • src/js/internal/repl/history.js
  • src/js/internal/repl/node-errors.js
  • src/js/internal/repl/node-inspect.js
  • src/js/internal/repl/node-primordials.js
  • src/js/internal/repl/node-shims.js
  • src/js/internal/repl/utils.js
  • src/js/internal/util/inspect.js
  • src/js/node/readline.js
  • src/js/node/readline.promises.js
  • src/js/node/readline.promises.ts
  • src/js/node/readline.ts
  • src/js/node/repl.js
  • src/js/node/repl.ts
  • src/jsc/bindings/NodeVMScript.cpp
  • src/options_types/context.rs
  • src/resolve_builtins/HardcodedModule.rs
  • src/runtime/cli/Arguments.rs
  • src/runtime/cli/mod.rs
  • src/runtime/cli/run_command.rs
  • src/runtime/jsc_hooks.rs
  • test/expectations.txt
  • test/js/node/test/common/repl.js
  • test/js/node/test/parallel/test-readline-promises-tab-complete.js
  • test/js/node/test/parallel/test-readline-tab-complete.js
  • test/js/node/test/parallel/test-repl-array-prototype-tempering.js
  • test/js/node/test/parallel/test-repl-async-iife.js
  • test/js/node/test/parallel/test-repl-autocomplete.js
  • test/js/node/test/parallel/test-repl-autolibs.js
  • test/js/node/test/parallel/test-repl-clear-immediate-crash.js
  • test/js/node/test/parallel/test-repl-cli-eval.js
  • test/js/node/test/parallel/test-repl-colors.js
  • test/js/node/test/parallel/test-repl-completion-on-getters-disabled.js
  • test/js/node/test/parallel/test-repl-context.js
  • test/js/node/test/parallel/test-repl-custom-eval-previews.js
  • test/js/node/test/parallel/test-repl-custom-eval.js
  • test/js/node/test/parallel/test-repl-definecommand.js
  • test/js/node/test/parallel/test-repl-domain.js
  • test/js/node/test/parallel/test-repl-editor.js
  • test/js/node/test/parallel/test-repl-empty.js
  • test/js/node/test/parallel/test-repl-end-emits-exit.js
  • test/js/node/test/parallel/test-repl-envvars.js
  • test/js/node/test/parallel/test-repl-eval-error-after-close.js
  • test/js/node/test/parallel/test-repl-function-definition-edge-case.js
  • test/js/node/test/parallel/test-repl-harmony.js
  • test/js/node/test/parallel/test-repl-history-dedup-multiline.js
  • test/js/node/test/parallel/test-repl-history-init-fail-leak.js
  • test/js/node/test/parallel/test-repl-history-navigation.js
  • test/js/node/test/parallel/test-repl-history-perm.js
  • test/js/node/test/parallel/test-repl-import-referrer.js
  • test/js/node/test/parallel/test-repl-inspect-defaults.js
  • test/js/node/test/parallel/test-repl-inspector.js
  • test/js/node/test/parallel/test-repl-let-process.js
  • test/js/node/test/parallel/test-repl-load-multiline-from-history.js
  • test/js/node/test/parallel/test-repl-load-multiline-no-trailing-newline.js
  • test/js/node/test/parallel/test-repl-load-multiline.js
  • test/js/node/test/parallel/test-repl-mode.js
  • test/js/node/test/parallel/test-repl-multiline-navigation-while-adding.js
  • test/js/node/test/parallel/test-repl-multiline-navigation.js
  • test/js/node/test/parallel/test-repl-multiline.js
  • test/js/node/test/parallel/test-repl-multiple-instances-async-error.js
  • test/js/node/test/parallel/test-repl-no-terminal-restore-process-listeners.js
  • test/js/node/test/parallel/test-repl-no-terminal.js
  • test/js/node/test/parallel/test-repl-null-thrown.js
  • test/js/node/test/parallel/test-repl-null.js
  • test/js/node/test/parallel/test-repl-options.js
  • test/js/node/test/parallel/test-repl-permission-model.js
  • test/js/node/test/parallel/test-repl-persistent-history.js
  • test/js/node/test/parallel/test-repl-preprocess-top-level-await.js
  • test/js/node/test/parallel/test-repl-pretty-custom-stack.js
  • test/js/node/test/parallel/test-repl-pretty-stack-custom-writer.js
  • test/js/node/test/parallel/test-repl-pretty-stack.js
  • test/js/node/test/parallel/test-repl-preview-newlines.js
  • test/js/node/test/parallel/test-repl-preview-timeout.js
  • test/js/node/test/parallel/test-repl-preview.js
  • test/js/node/test/parallel/test-repl-programmatic-history-setup-history.js
  • test/js/node/test/parallel/test-repl-programmatic-history.js
  • test/js/node/test/parallel/test-repl-recoverable.js
  • test/js/node/test/parallel/test-repl-require-after-write.js
  • test/js/node/test/parallel/test-repl-require-cache.js
  • test/js/node/test/parallel/test-repl-require-context.js
  • test/js/node/test/parallel/test-repl-require-self-referential.js
  • test/js/node/test/parallel/test-repl-require.js
  • test/js/node/test/parallel/test-repl-reset-event.js
  • test/js/node/test/parallel/test-repl-reverse-search.js
  • test/js/node/test/parallel/test-repl-save-load-editor-mode.js
  • test/js/node/test/parallel/test-repl-save-load-invalid-save.js
  • test/js/node/test/parallel/test-repl-save-load-load-dir.js
  • test/js/node/test/parallel/test-repl-save-load-load-non-existent.js
  • test/js/node/test/parallel/test-repl-save-load-load-without-name.js
  • test/js/node/test/parallel/test-repl-save-load-save-without-name.js
  • test/js/node/test/parallel/test-repl-save-load.js
  • test/js/node/test/parallel/test-repl-setprompt.js
  • test/js/node/test/parallel/test-repl-sigint-nested-eval.js
  • test/js/node/test/parallel/test-repl-sigint.js
  • test/js/node/test/parallel/test-repl-stdin-push-null.js
  • test/js/node/test/parallel/test-repl-strict-mode-previews.js
  • test/js/node/test/parallel/test-repl-syntax-error-stack.js
  • test/js/node/test/parallel/test-repl-tab-complete-buffer.js
  • test/js/node/test/parallel/test-repl-tab-complete-computed-props.js
  • test/js/node/test/parallel/test-repl-tab-complete-crash.js
  • test/js/node/test/parallel/test-repl-tab-complete-custom-completer.js
  • test/js/node/test/parallel/test-repl-tab-complete-files.js
  • test/js/node/test/parallel/test-repl-tab-complete-import.js
  • test/js/node/test/parallel/test-repl-tab-complete-nested-repls.js
  • test/js/node/test/parallel/test-repl-tab-complete-new-expression.js
  • test/js/node/test/parallel/test-repl-tab-complete-no-warn.js
  • test/js/node/test/parallel/test-repl-tab-complete-nosideeffects.js
  • test/js/node/test/parallel/test-repl-tab-complete-on-editor-mode.js
  • test/js/node/test/parallel/test-repl-tab-complete-require.js
  • test/js/node/test/parallel/test-repl-tab-complete-unary-expressions.js
  • test/js/node/test/parallel/test-repl-tab-complete.js
  • test/js/node/test/parallel/test-repl-tab.js
  • test/js/node/test/parallel/test-repl-throw-null-or-undefined.js
  • test/js/node/test/parallel/test-repl-top-level-await.js
  • test/js/node/test/parallel/test-repl-uncaught-exception-after-input-ended.js
  • test/js/node/test/parallel/test-repl-uncaught-exception-async.js
  • test/js/node/test/parallel/test-repl-uncaught-exception-evalcallback.js
  • test/js/node/test/parallel/test-repl-uncaught-exception-standalone.js
  • test/js/node/test/parallel/test-repl-uncaught-exception.js
  • test/js/node/test/parallel/test-repl-underscore.js
  • test/js/node/test/parallel/test-repl-unexpected-token-recoverable.js
  • test/js/node/test/parallel/test-repl-unsafe-array-iteration.js
  • test/js/node/test/parallel/test-repl-unsupported-option.js
  • test/js/node/test/parallel/test-repl-use-global.js
  • test/js/node/test/parallel/test-repl-user-error-handler.js
  • test/js/node/test/parallel/test-repl.js
  • test/js/node/test/sequential/test-repl-timeout-throw.js
  • test/js/node/util/node-inspect-tests/parallel/util-inspect.test.js
💤 Files with no reviewable changes (19)
  • test/js/node/test/parallel/test-repl-unsupported-option.js
  • test/js/node/test/parallel/test-repl-uncaught-exception-evalcallback.js
  • test/js/node/test/parallel/test-repl-uncaught-exception-after-input-ended.js
  • test/js/node/test/parallel/test-repl-uncaught-exception-async.js
  • test/js/node/test/parallel/test-repl-tab.js
  • src/js/node/readline.promises.ts
  • test/js/node/test/parallel/test-repl-uncaught-exception-standalone.js
  • test/js/node/test/parallel/test-repl-throw-null-or-undefined.js
  • src/js/node/repl.ts
  • test/js/node/test/parallel/test-repl-unexpected-token-recoverable.js
  • test/js/node/test/parallel/test-repl-use-global.js
  • test/js/node/test/parallel/test-repl-unsafe-array-iteration.js
  • test/js/node/test/parallel/test-repl-uncaught-exception.js
  • src/js/node/readline.ts
  • test/js/node/test/parallel/test-repl-tab-complete-unary-expressions.js
  • test/js/node/test/parallel/test-repl-underscore.js
  • test/js/node/test/parallel/test-repl-tab-complete.js
  • test/js/node/test/parallel/test-repl-top-level-await.js
  • test/js/node/test/parallel/test-repl-user-error-handler.js

Comment thread src/js/internal/readline/interface.js
Comment thread src/js/internal/repl/await.js
Comment thread src/js/internal/repl/node-primordials.js
Comment thread src/js/node/repl.js Outdated
Comment thread test/js/node/test/parallel/test-repl-persistent-history.js
Comment thread test/expectations.txt Outdated
Comment thread src/js/node/readline.js

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
src/js/node/readline.js (1)

531-544: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Use a primordial-safe property helper here.

These two Object.defineProperty() calls read from the user-overridable global Object. Since node:readline can be loaded after user code has run, a patched global can change or break module initialization. Prefer the captured primordial helper for both definitions.

♻️ Suggested change
-Object.defineProperty(__node_module__.exports, Symbol.for("__BUN_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED__"), {
+ObjectDefineProperty(__node_module__.exports, Symbol.for("__BUN_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED__"), {
   __proto__: null,
   value: {
     CSI: require("internal/readline/utils").CSI,
     utils: {
       getStringWidth: require("internal/util/inspect").getStringWidth,
       stripVTControlCharacters: require("node:util").stripVTControlCharacters,
     },
   },
 });
@@
-Object.defineProperty(Interface.prototype.question[promisify.custom], "name", { value: "question" });
+ObjectDefineProperty(Interface.prototype.question[promisify.custom], 'name', { value: 'question' });

Also add ObjectDefineProperty to the primordials destructure at the top of the file.

As per coding guidelines, built-in JS modules are hot-path code in a hostile environment and should use primordial-safe calls instead of user-overridable globals.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/js/node/readline.js` around lines 531 - 544, Replace the global,
user-overridable Object.defineProperty calls with the primordial-safe helper
(ObjectDefineProperty) used elsewhere: update the two calls that set
Symbol.for("__BUN_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED__") on
__node_module__.exports and the call that sets the name on
Interface.prototype.question[promisify.custom] to use ObjectDefineProperty
instead of Object.defineProperty; also add ObjectDefineProperty to the
primordials destructure at the top of the file so the helper is available.
src/js/node/readline.promises.js (1)

66-67: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Normalize 1-arg completers before constructing the promises interface.

src/js/node/readline.js:106-126 wraps sync completers into the 2-arg callback shape before it calls the internal constructor. createInterface() here bypasses that step, so readline.promises.createInterface() can behave differently for the same completer input.

Suggested fix
 function createInterface(input, output, completer, terminal) {
+  if (input?.input &&
+      typeof input.completer === 'function' &&
+      input.completer.length !== 2) {
+    const { completer } = input;
+    input = {
+      ...input,
+      completer: (v, cb) => cb(null, completer(v)),
+    };
+  } else if (typeof completer === 'function' && completer.length !== 2) {
+    const realCompleter = completer;
+    completer = (v, cb) => cb(null, realCompleter(v));
+  }
+
   return new Interface(input, output, completer, terminal);
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/js/node/readline.promises.js` around lines 66 - 67, createInterface in
readline.promises constructs the Interface directly and bypasses the
normalization of 1-arg completers that readline.js does (the sync-to-2-arg
callback wrapper used in readline.js lines ~106-126), which causes different
behavior for the same completer input; fix it by applying the same completer
normalization before constructing the Interface in createInterface (i.e., wrap a
single-argument sync completer into the expected (line, callback) shape or reuse
the existing normalization helper used by readline.js) so that createInterface
and Interface receive the normalized 2-arg completer.
src/js/internal/repl/history.js (1)

326-333: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don't leave flushHistory waiters hanging on write failure.

initialize() pauses the interface and waits for the first flushHistory before resuming. In kFlushHistory(), the catch branch only logs, so a write failure leaves startup paused forever and can also stall the exit path that's waiting on the same event.

Suggested fix
   } catch (err) {
     this[kWriting] = false;
+    if (this[kPending]) {
+      this[kPending] = false;
+      this[kOnLine]();
+    } else {
+      this[kIsFlushing] = Boolean(this[kTimer]);
+      if (!this[kIsFlushing]) {
+        this[kContext].emit('flushHistory');
+      }
+    }
     debug('Error writing history file:', err);
   }

As per coding guidelines, “Every error/abort/timeout path actively completes the operation.”

Also applies to: 367-393, 396-399

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/js/internal/repl/history.js` around lines 326 - 333, The start-up can
hang because kFlushHistory() only logs write errors and never signals the
'flushHistory' waiter; update kFlushHistory() (and the other catch branches
referenced around lines 367-393 and 396-399) so that on any failure or abort it
actively emits or otherwise triggers the same completion event the initialize()
waiter is listening for (the kContext 'flushHistory' event) and cleans up (e.g.,
ensure kContext.emit('flushHistory') or equivalent is called and any paused
state is cleared) so initialize()'s once('flushHistory', ...) callback always
runs even on write failure.
♻️ Duplicate comments (1)
src/js/node/repl.js (1)

1515-1518: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Use the captured primordial for the standalone export hook.

This is the one remaining Object.defineProperty(...) call in a src/js/ builtin. If userland mutates the global before require("node:repl"), this hook stops following the file’s tamper-resistant pattern.

Suggested fix
-Object.defineProperty(__node_module__.exports, Symbol.for("bun.repl.kStandaloneREPL"), {
+ObjectDefineProperty(__node_module__.exports, Symbol.for("bun.repl.kStandaloneREPL"), {
   __proto__: null,
   value: kStandaloneREPL,
 });

As per coding guidelines, built-in JS modules (src/js/) are hot-path code in a hostile environment and should use captured/private globals rather than public JavaScript globals.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/js/node/repl.js` around lines 1515 - 1518, The Object.defineProperty call
uses public globals; replace it with the captured primordials for defineProperty
and Symbol.for so the standalone export hook remains tamper-resistant.
Specifically, change the call that references Object.defineProperty and
Symbol.for("bun.repl.kStandaloneREPL") to use the captured primitives (e.g.,
capturedDefineProperty or primordials.Object_defineProperty and
capturedSymbolFor or primordials.Symbol_for) when defining kStandaloneREPL on
__node_module__.exports; keep the same property descriptor (value:
kStandaloneREPL, __proto__: null) and the same target (__node_module__.exports)
and symbol key.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/js/internal/repl/history.js`:
- Around line 326-333: The start-up can hang because kFlushHistory() only logs
write errors and never signals the 'flushHistory' waiter; update kFlushHistory()
(and the other catch branches referenced around lines 367-393 and 396-399) so
that on any failure or abort it actively emits or otherwise triggers the same
completion event the initialize() waiter is listening for (the kContext
'flushHistory' event) and cleans up (e.g., ensure kContext.emit('flushHistory')
or equivalent is called and any paused state is cleared) so initialize()'s
once('flushHistory', ...) callback always runs even on write failure.

In `@src/js/node/readline.js`:
- Around line 531-544: Replace the global, user-overridable
Object.defineProperty calls with the primordial-safe helper
(ObjectDefineProperty) used elsewhere: update the two calls that set
Symbol.for("__BUN_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED__") on
__node_module__.exports and the call that sets the name on
Interface.prototype.question[promisify.custom] to use ObjectDefineProperty
instead of Object.defineProperty; also add ObjectDefineProperty to the
primordials destructure at the top of the file so the helper is available.

In `@src/js/node/readline.promises.js`:
- Around line 66-67: createInterface in readline.promises constructs the
Interface directly and bypasses the normalization of 1-arg completers that
readline.js does (the sync-to-2-arg callback wrapper used in readline.js lines
~106-126), which causes different behavior for the same completer input; fix it
by applying the same completer normalization before constructing the Interface
in createInterface (i.e., wrap a single-argument sync completer into the
expected (line, callback) shape or reuse the existing normalization helper used
by readline.js) so that createInterface and Interface receive the normalized
2-arg completer.

---

Duplicate comments:
In `@src/js/node/repl.js`:
- Around line 1515-1518: The Object.defineProperty call uses public globals;
replace it with the captured primordials for defineProperty and Symbol.for so
the standalone export hook remains tamper-resistant. Specifically, change the
call that references Object.defineProperty and
Symbol.for("bun.repl.kStandaloneREPL") to use the captured primitives (e.g.,
capturedDefineProperty or primordials.Object_defineProperty and
capturedSymbolFor or primordials.Symbol_for) when defining kStandaloneREPL on
__node_module__.exports; keep the same property descriptor (value:
kStandaloneREPL, __proto__: null) and the same target (__node_module__.exports)
and symbol key.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1963343f-8367-4d39-8ad4-16a46847ebdb

📥 Commits

Reviewing files that changed from the base of the PR and between dfc8580 and e417315.

📒 Files selected for processing (14)
  • src/js/internal/readline/callbacks.js
  • src/js/internal/readline/emitKeypressEvents.js
  • src/js/internal/readline/interface.js
  • src/js/internal/readline/promises.js
  • src/js/internal/readline/utils.js
  • src/js/internal/repl.js
  • src/js/internal/repl/completion.js
  • src/js/internal/repl/history.js
  • src/js/internal/repl/utils.js
  • src/js/node/readline.js
  • src/js/node/readline.promises.js
  • src/js/node/repl.js
  • test/expectations.txt
  • test/no-validate-exceptions.txt

cirospaciari and others added 12 commits June 5, 2026 20:25
…tests

Replaces the node:repl stub with a full port of Node v26.3.0's repl
implementation, including the internal/readline stack it depends on:
ported repl.js + internal/repl/{utils,completion,history,await}, the v26
internal/readline stack, vendored acorn (vm-evaluated), Node-internals
shims, an --interactive CLI flag booting the Node-compatible REPL, and
debug-build exposure of internal/repl* for the vendored Node test suite.
Also: vm per-call displayErrors gates stack decoration; inspect
recognizes cross-realm errors; vendored all 107 node repl tests.

Known adaptations: contextified sandbox instead of DONT_CONTEXTIFY (var
persistence), eager parse via createCachedData (lazy vm syntax errors),
decorateErrorStack reproduces Node's REPL frame trimming (eager JSC
stacks).
…pending-deprecation

- ERR_* classes: decorated name in stack header only (Node resets .name
  to the base class after stack capture); captureStackTrace hides ctors.
- decorateErrorStack: cut at the last bare REPL-resource frame (Node's
  null-functionName boundary), normalize JSC anonymous frames to V8 form.
- context lazy builtin libs: include repl/domain/sys like Node.
- --interactive entry sets repl.repl introspection property.
- getOptionValue('--pending-deprecation') reads process.execArgv.
The ported repl/readline files stay byte-close to upstream Node, so they
go in ignorePatterns like other vendored code; the hand-written shims get
real fixes (underscore-prefixed unused params, drop an unused require).
…placement

- node-errors.js: the default-formatting fallback now iterates the
  null-guarded frames, matching the override/prev branches.
- run_command.rs: move exec_node_repl below exec_eval so exec_eval's
  doc comment attaches to the right item.
…iler limits

The asan build (the only ASSERT_ENABLED cpp job) failed compiling
codegen/InternalModuleRegistryConstants.h: the vendored acorn dist
embedded a 245KB string literal, 2.25x larger than any other module's.
Minifying the dist with esbuild before wrapping brings it to 122KB,
in line with the largest pre-existing literal (node:http2 at 108KB).
- cascadedLoader.import resolves relative specifiers against the
  threaded parentURL (cwd/repl) instead of the bundled module.
- The capture-callback dispatcher falls through to the regular
  'uncaughtException' flow before the fatal handler, matching Node's
  additive-API semantics.
- getSchemeOnlyModuleNames returns bare names; completion.js adds the
  node: prefix itself.
The x64-asan build (the only ASSERT_ENABLED cpp job) failed compiling
codegen/InternalModuleRegistryConstants.h: with assertions on,
ASCIILiteral::fromLiteralUnsafe constexpr-evaluates the embedded module
source character by character, and the vendored acorn literal (122KB,
vs node:http2's previous 108KB maximum) exceeded the old step budget.
…ntics, expectations

- readline.js re-exports the __BUN_INTERNALS symbol (CSI, getStringWidth,
  stripVTControlCharacters) that pre-existing readline tests consume;
  getStringWidth is now exported from internal/util/inspect.
- bun --interactive foo.js runs the script (Node -i semantics: the REPL
  only starts when no script is given).
- The --interactive entry opts into standalone-REPL semantics via a
  private symbol on the repl module (kStandaloneREPL): relaxed input
  validation, repl.repl introspection, replDefaults writer wiring.
- test-readline-promises-tab-complete re-synced toward upstream
  (fi.end() instead of the close-race rli.close()) with macrotask waits
  for JSC's await-chain scheduling.
- Remaining 25 vendored repl divergences registered in
  test/expectations.txt (82/107 passing).
robobun added a commit that referenced this pull request Jul 26, 2026
The +550 KB vs canary #79916 comes from the 11 main commits between that
baseline and this branch's merge-base (916492f), notably quic
(#32602), node:repl (#31827), node:inspector (#31823), and the TLS work
(#34598). This PR's own diff is ~130 net lines across two .rs files.
robobun added a commit that referenced this pull request Jul 26, 2026
…ctive, dedupe watch.test.ts waitFor

c-bindings: the SIG_DFL reset loop now queries the old disposition and
skips SIG_IGN/SIG_DFL, so an inherited SIG_IGN (nohup SIGHUP, job-control
SIGTTIN/SIGTTOU) survives the reload like it did before execve. The loop
only needs to reset caught handlers (the queued-and-lost case); SIG_IGN
never enters Bun__onPosixSignal.

Arguments: main's #31827 added a visible --interactive entry at :182, so
this PR's hidden one at :319 was dead after the merge. Removed.

watch.test.ts: the three byte-identical reader/decoder/waitFor blocks are
now a single stdoutWaiter() helper; stderr goes to inherit in the four
tests that were piping-and-ignoring it.
robobun added a commit that referenced this pull request Jul 26, 2026
The binary-size baseline is build #79916 (ae4b17d, 2026-07-25). Since
then 12 commits landed on main including node:quic (#32602), node:repl
(#31827), node:inspector (#31823) and the tls overhaul (#34598); other
PRs branched from current main see the same ~550KB delta (e.g. build
82225). This PR adds two small node:fs ops.
Jarred-Sumner pushed a commit that referenced this pull request Jul 27, 2026
… slowly (#36006)

## Repro

A `Bun.serve` handler that reads `req.body` slower than the client sends
gets no TCP backpressure: the client is never throttled and the whole
remaining body is buffered in server memory.

```js
const s = Bun.serve({
  port: 0,
  maxRequestBodySize: 2 ** 33,
  async fetch(req) {
    let n = 0, mx = 0;
    for await (const c of req.body) {
      n += c.length; mx = Math.max(mx, c.length);
      await Bun.sleep(n / 5e6 * 1000 - performance.now()); // ~5 MB/s sink
    }
    return Response.json({ bytes: n, maxChunkMB: Math.round(mx / 1e6) });
  },
});
// raw-socket client PUTs 60 MB at loopback speed
```

Before: `{"clientWriteMs":116}` / server
`{"bytes":60000000,"maxChunkMB":55,"peakRssMB":204}` (client dumps 60 MB
in 116 ms; one 55 MB chunk).
After: `{"clientWriteMs":10586}` / server `{"maxChunkMB":1}` (client
throttled to the sink rate; every chunk ≤ ~1.4 MB).

The same shape held for `pipeTo(slowWritable)`, for `getReader()` + one
`read()` then idle, and for a handler that did not touch `req.body` at
all while the body arrived. Bun's own `node:http` server already applied
backpressure on the same uWS socket (`NodeHTTPResponse::pause_socket`),
so the primitive was in place; the `Bun.serve` `req.body` path just
never used it.

## Cause

`RequestContext::on_buffered_body_chunk` forwards every uWS `onData`
chunk into `ByteStream::on_data`. When no JS reader is waiting,
`on_data` appends to the ByteStream's internal `buffer: Vec<u8>` and
returns; nothing observed that buffer's size and nothing called
`resp.pause()`. For a body that has not been touched yet the chunk is
appended to `request_body_buf` with the same unbounded growth.

## Fix

**Pause.** `on_buffered_body_chunk` pauses the socket once the
ByteStream's unconsumed buffer (or the pre-stream `request_body_buf`)
crosses a 1 MiB high-water mark. The ByteStream's existing
`signal_drained` (fired from `on_pull` when the buffer empties) is wired
to resume via `on_stream_drained` on the request's `PendingValue`.

**Whole-body consumers.** A consumer that wants the whole body never
drives `on_pull`, so pausing would wedge it:
- `.text()`/`.json()`/`.arrayBuffer()` on an untouched body fire
`on_start_buffering`, which resumes and sets `REQUEST_BODY_BUFFER_ALL`
to suppress further pre-stream pausing.
- `.text()` after `req.body` has been touched goes through the
ByteStream's `buffer_action`; `on_buffered_body_chunk` skips the pause
when `buffer_action` or a native `pipe` is set.
- `Bun.write(file, req)` registered `on_receive_value` without calling
`on_start_buffering`; it now does, mirroring `BodyValueBufferer`.

**Stale-`resp` window.** Once a streaming-response sink calls
`res.end()`, uWS `markDone()` drops `onAborted` and (for `Connection:
close`) the socket may be freed on the next loop tick while the
`RequestContext` still holds `resp`. The sink resumes the socket at both
points it sets `ended_response = true` (same frame as `res.end()`, so
the socket is at worst closed, never freed). Every Rust-side resume path
consults `resp_may_be_freed()` (i.e. `sink.ended_response`) and clears
the flag without dereferencing `resp` once the sink has ended.
`handle_resolve_stream`/`handle_reject_stream` clear
`REQUEST_BODY_PAUSED` and the ByteStream `drain_handler` immediately
after reading `ended_response`, before `detach()`/`run_error_handler`
can re-enter JS. `end_already_responded_stream` and `detach_response`
clear the flag without dereferencing. uWS itself is unchanged, so
node:http's own pause owners (`pausePipelineReads`, `IncomingMessage`
pause, the C++ pipeline-flood guard) are unaffected.

Two `FlagsBits` are added (widening the set to `u32`):
`REQUEST_BODY_PAUSED` and `REQUEST_BODY_BUFFER_ALL`.

## Verification

`bun bd test test/js/bun/http/serve.test.ts -t "request body
backpressure"`

Five tests next to the existing response-side backpressure tests: a
stalled streaming reader, a handler that defers touching `req.body`,
`Bun.write(file, req)` after the pre-stream pause, and `.arrayBuffer()`
with and without `req.body` touched first. Each writes a 32 MiB body
from a raw socket, polls the client's `sent` counter until it plateaus,
and asserts the plateau is well short of the total and the largest chunk
the handler sees is under 4 MiB. All five fail on `main` and pass with
this change; bytes are delivered intact.

The stale-`resp` paths (reader.read and req.text inside a direct-stream
`pull()` after `c.end()` across a loop tick) were verified under ASAN.
`node-http.test.ts` "pipelined responses buffered past the high water
mark pause reads on the connection" passes.

## Binary size

The +~530 KB flagged by the size check is measured against main build
79916, the last passing canary. Twelve commits landed on main between
that baseline and this PR's parent, including `node:quic` (#32602), the
full `node:repl` (#31827), and `node:inspector` (#31823), none of which
has produced a passing canary yet.

<!-- robobun:evidence:begin -->

---

**no test proof** · iteration 2 · Platform-specific test(s) that do not
run on this machine. Deferring to CI, which covers all platforms:
test/js/bun/http/serve.test.ts

<!-- robobun:evidence:end -->
Jarred-Sumner pushed a commit that referenced this pull request Jul 27, 2026
…tack, acorn recoverable-parse + top-level await, completion, history, --interactive (82 vendored upstream tests) (#31827)

Brings `node:repl` from a stub that threw on use to a working port of
Node's REPL, by porting the upstream implementation
verbatim-where-possible and vendoring the upstream test suite.

**82 of Node v26.3.0's 107 upstream REPL tests pass and are vendored
here.** The other 25 do not pass yet, so they are **not** committed — a
vendored failing test needs an `expectations.txt` entry to keep CI
green, which adds no coverage and hides the gap. They are enumerated
under "Known gaps" below as follow-up work. This branch does not touch
`test/expectations.txt`.

### What's implemented

- **The full `node:repl` module** ported from Node v26.3.0:
`repl.start()`, `REPLServer`, `Recoverable`, `REPL_MODE_SLOPPY/STRICT`,
`builtinModules`, the `writer`/`eval` option contracts,
`.break`/`.clear`/`.editor`/`.exit`/`.help`/`.save`/`.load` commands,
`_`/`_error` underscore assignment, and `inspect.replDefaults`
passthrough.
- **The v26 `internal/readline` stack** (`interface`, `utils`,
`callbacks`, `emitKeypressEvents`, `promises`) replaces the older
readline port, so `node:readline` and `node:readline/promises` are the
same code Node ships; multiline editing, history navigation,
reverse-i-search and keypress decoding come from upstream.
- **Recoverable-error detection and top-level `await`** via acorn +
acorn-walk vendored from Node's deps and evaluated with `vm.Script` (JSC
builtin functions are non-constructors, which acorn's ES5 prototype
style can't live under); `const foo = {` continues with `| ` exactly
like Node, and `await` input is rewritten through
`processTopLevelAwait`.
- **Tab completion** (`completer`, `allowBlockingCompletions`,
member-expression evaluation with proxy/getter safety) and **persistent
history** (`setupHistory`, `NODE_REPL_HISTORY`, dedup, size limits).
- **A `--interactive` CLI flag** that boots the Node-compatible REPL
(banner, `NODE_REPL_*` env handling, history file, `repl.repl`
introspection). Bun's own `-i` is `--install=fallback`, so the long form
is used for `bun`; under node emulation (argv0 `node`) `-i` means
`--interactive`, as it does in Node.
- **`-e` under `--interactive` matches `node -i -e`**: the script is
evaluated *after* `createInternalRepl` (which starts the REPL
synchronously), through the equivalent of Node's `runScriptInContext` —
the CJS bindings are published onto the global and the body runs in
global scope, so `require`/`module`/`exports`/`__filename`/`__dirname`
resolve while `var`/`function` declarations still land on `globalThis`.
An `-e` error stays fatal (exit 1, stdin never read).
- **Node's REPL error presentation**: `Uncaught <Error>` formatting,
REPL-frame trimming at the eval boundary, `ERR_INVALID_REPL_INPUT`-style
decorated stack headers, `ERR_*` `instanceof` semantics, and cross-realm
(vm-context) errors now render with name/message/stack through
`util.inspect` (previously printed as `ReferenceError {}`).
- **`internal/repl`, `internal/repl/await`, `internal/repl/history`,
`internal/util/inspect`** are requirable in debug builds, matching what
the upstream suite reaches via `--expose-internals`.

### Native fixes the suite surfaced

- **vm**: per-call `displayErrors` now gates the error-stack source-line
decoration (Node only decorates when `displayErrors !== false`; we
decorated unconditionally, so REPL syntax errors carried a spurious
`evalmachine.<anonymous>:1` prefix).
- **vm**: `new vm.Script(bad)` now throws the `SyntaxError` at
construction like Node, via an eager `JSC::checkSyntax` (Bun deferred it
to run time). The REPL's recoverable-error flow depends on the eager
throw. Noted as a double-parse to fold into compile-once in a follow-up.
- **vm**: `vm.compileFunction` now gets the same arrow header as `new
vm.Script` (Node's `DecorateErrorStack` runs for both). The header URL
is resolved by the caller rather than inferred from an empty filename,
because `ScriptOptions` seeds `filename` with an empty *non-null* string
— so emptiness cannot distinguish "absent" from an explicit `{ filename:
"" }`. That also fixes `new vm.Script(src, { filename: "" })`, which
rendered `evalmachine.<anonymous>:1` where Node renders `:1`.
- **readline**: `Interface.prototype[Symbol.dispose].name` is the string
`"[Symbol.dispose]"`, matching upstream's `assignFunctionName`; it was
the raw Symbol, so `util.inspect` differed and any coercion of `.name`
threw.
- **async_hooks**: `AsyncLocalStorage.run`/`snapshot` called the
callback with a spread, which routes through
`Array.prototype[Symbol.iterator]`. Userland can delete that, and the
REPL wraps every eval in `replContext.run()`, so deleting it made the
REPL report `Spread syntax requires ...` instead of the user's own
error. Node uses `ReflectApply` here for the same reason. (`bind`/`exit`
have the same latent call-site spread and are left for a focused
async_hooks change.)
- **inspect**: errors from other realms (vm contexts) are recognized via
`isNativeError` instead of `instanceof Error`.
- **builtins codegen**: a regex literal at statement start silently
truncated the rest of the bundled module — surfaced by
`internal/repl/await`, worked around by hoisting; the codegen bug is a
follow-up.

### Vendored tests

82 upstream test files, byte-identical to Node v26.3.0 apart from two
kinds of deviation, each carrying an inline `bun:` comment:

- **`-i` → `--interactive`** (8 files:
`test-repl-clear-immediate-crash`, `test-repl-harmony`,
`test-repl-inspect-defaults`, `test-repl-require-after-write`,
`test-repl-sigint`, `test-repl-sigint-nested-eval`,
`test-repl-uncaught-exception-standalone`,
`test-repl-unexpected-token-recoverable`) — these spawn
`process.execPath`, which is `bun`, not the node shim, so bun's `-i`
(`--install=fallback`) applies and the long form is used instead.
- `test-readline-promises-tab-complete.js` waits a macrotask per
keystroke instead of a microtask; JSC settles the completion await chain
across more microtask turns than V8. The assertions are unchanged.

`test-readline-tab-complete.js` had a Bun-weakened assertion (`/^Tab
completion error:[^]+error: message/`, commented "modified to match
bun's error message"); it is **restored to the upstream text** since the
ported readline now produces Node's message.

Two upstream fixtures are force-added
(`test/js/node/test/fixtures/repl-load-multiline.js`,
`repl-tab-completion-nested-repls.js`): `test/js/node/test/.gitignore`
excludes `fixtures/repl*` alongside the other upstream fixture trees
this port never vendored, an exclusion that predates vendoring any REPL
tests.

### Known gaps (25 upstream tests not vendored)

Result previews need inspector-backed side-effect-free evaluation
(`Runtime.evaluate` with `throwOnSideEffect`); JSC's inspector has no
equivalent wired up, and `sendInspectorCommand` is a stub:
`test-repl-custom-eval-previews` · `test-repl-history-navigation` ·
`test-repl-preview` · `test-repl-preview-newlines` ·
`test-repl-reverse-search` · `test-repl-strict-mode-previews` ·
`test-repl-mode` · `test-repl-inspector`

JSC-vs-V8 stack frame format / error wording (`Unexpected end of script`
vs `Unexpected end of input`, `at readdirSync` vs `at
Object.readdirSync`, `{}` vs `[3,2,1]` in "is not iterable"), and
message *lengths* that flip Node's own `breakLength: 80` heuristic:
`test-repl-pretty-stack` · `test-repl-pretty-custom-stack` ·
`test-repl-user-error-handler` · `test-repl-top-level-await` ·
`test-repl-unsafe-array-iteration` · `test-repl-underscore`

SIGINT during eval does not interrupt vm execution yet:
`test-repl-timeout-throw`

Tab completion edge cases — `test-repl-tab-complete` needs
`Runtime.globalLexicalScopeNames` (a V8 protocol method with no JSC
counterpart) for `let`/`const`/`class` completion;
`test-repl-tab-complete-require` and `-import` additionally assert that
`./test-repl-tab-complete.js` exists in the test directory, so they can
only be vendored once it is:
`test-repl-tab-complete` · `test-repl-tab-complete-unary-expressions` ·
`test-repl-tab-complete-require` · `test-repl-tab-complete-import`

Assorted one-offs:
`test-repl` (large integration test; 250/265 cases pass, the remainder
are V8-message-text assertions) · `test-repl-require` (module resolution
edge cases) · `test-repl-domain` (node:domain integration incomplete) ·
`test-repl-import-referrer` (JSC renders module namespaces differently)
· `test-repl-unsupported-option` (`--input-type` validation not
implemented) · `test-repl-uncaught-exception-async` (uncaughtException
listener guard timing)

### Known limitations / follow-ups

- **`vm.constants.DONT_CONTEXTIFY` drops `var` persistence across script
runs** (lexical declarations survive, `var` doesn't), so the REPL
context uses a contextified sandbox — Node's pre-v22 behavior — until
the native context is fixed.
- **`process.addUncaughtExceptionCaptureCallback` /
`removeUncaughtExceptionCaptureCallback`** don't exist in Bun; shimmed
over the single-callback set/clear API inside the REPL only.
- Stacks materialize eagerly under JSC, so Node's
`overrideStackTrace`-based REPL frame trimming is reproduced in
`decorateErrorStack` instead of at capture time.
- `process.config.variables.v8_enable_i8n_support` is misspelled (Node
reads `v8_enable_i18n_support`), so `common.hasIntl` is always false and
the suite silently skips its Intl-gated assertions. Out of scope here —
it affects ~45 vendored tests across the tree and deserves its own
change.

Fixes #28478

<!-- robobun:evidence:begin -->

---

**no test proof** · iteration 16 · Platform-specific test(s) that do not
run on this machine. Deferring to CI, which covers all platforms:
test/cli/run/run-eval.test.ts test/js/bun/repl/repl.test.ts
test/js/node/util/node-inspect-tests/parallel/util-inspect.test.js

<!-- robobun:evidence:end -->

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: robobun <robobun@oven.sh>
Co-authored-by: Alistair Smith <hi@alistair.sh>
Co-authored-by: Dylan Conway <dylan.conway567@gmail.com>
Co-authored-by: robobun <117481402+robobun@users.noreply.github.com>
Jarred-Sumner pushed a commit that referenced this pull request Jul 27, 2026
… slowly (#36006)

## Repro

A `Bun.serve` handler that reads `req.body` slower than the client sends
gets no TCP backpressure: the client is never throttled and the whole
remaining body is buffered in server memory.

```js
const s = Bun.serve({
  port: 0,
  maxRequestBodySize: 2 ** 33,
  async fetch(req) {
    let n = 0, mx = 0;
    for await (const c of req.body) {
      n += c.length; mx = Math.max(mx, c.length);
      await Bun.sleep(n / 5e6 * 1000 - performance.now()); // ~5 MB/s sink
    }
    return Response.json({ bytes: n, maxChunkMB: Math.round(mx / 1e6) });
  },
});
// raw-socket client PUTs 60 MB at loopback speed
```

Before: `{"clientWriteMs":116}` / server
`{"bytes":60000000,"maxChunkMB":55,"peakRssMB":204}` (client dumps 60 MB
in 116 ms; one 55 MB chunk).
After: `{"clientWriteMs":10586}` / server `{"maxChunkMB":1}` (client
throttled to the sink rate; every chunk ≤ ~1.4 MB).

The same shape held for `pipeTo(slowWritable)`, for `getReader()` + one
`read()` then idle, and for a handler that did not touch `req.body` at
all while the body arrived. Bun's own `node:http` server already applied
backpressure on the same uWS socket (`NodeHTTPResponse::pause_socket`),
so the primitive was in place; the `Bun.serve` `req.body` path just
never used it.

## Cause

`RequestContext::on_buffered_body_chunk` forwards every uWS `onData`
chunk into `ByteStream::on_data`. When no JS reader is waiting,
`on_data` appends to the ByteStream's internal `buffer: Vec<u8>` and
returns; nothing observed that buffer's size and nothing called
`resp.pause()`. For a body that has not been touched yet the chunk is
appended to `request_body_buf` with the same unbounded growth.

## Fix

**Pause.** `on_buffered_body_chunk` pauses the socket once the
ByteStream's unconsumed buffer (or the pre-stream `request_body_buf`)
crosses a 1 MiB high-water mark. The ByteStream's existing
`signal_drained` (fired from `on_pull` when the buffer empties) is wired
to resume via `on_stream_drained` on the request's `PendingValue`.

**Whole-body consumers.** A consumer that wants the whole body never
drives `on_pull`, so pausing would wedge it:
- `.text()`/`.json()`/`.arrayBuffer()` on an untouched body fire
`on_start_buffering`, which resumes and sets `REQUEST_BODY_BUFFER_ALL`
to suppress further pre-stream pausing.
- `.text()` after `req.body` has been touched goes through the
ByteStream's `buffer_action`; `on_buffered_body_chunk` skips the pause
when `buffer_action` or a native `pipe` is set.
- `Bun.write(file, req)` registered `on_receive_value` without calling
`on_start_buffering`; it now does, mirroring `BodyValueBufferer`.

**Stale-`resp` window.** Once a streaming-response sink calls
`res.end()`, uWS `markDone()` drops `onAborted` and (for `Connection:
close`) the socket may be freed on the next loop tick while the
`RequestContext` still holds `resp`. The sink resumes the socket at both
points it sets `ended_response = true` (same frame as `res.end()`, so
the socket is at worst closed, never freed). Every Rust-side resume path
consults `resp_may_be_freed()` (i.e. `sink.ended_response`) and clears
the flag without dereferencing `resp` once the sink has ended.
`handle_resolve_stream`/`handle_reject_stream` clear
`REQUEST_BODY_PAUSED` and the ByteStream `drain_handler` immediately
after reading `ended_response`, before `detach()`/`run_error_handler`
can re-enter JS. `end_already_responded_stream` and `detach_response`
clear the flag without dereferencing. uWS itself is unchanged, so
node:http's own pause owners (`pausePipelineReads`, `IncomingMessage`
pause, the C++ pipeline-flood guard) are unaffected.

Two `FlagsBits` are added (widening the set to `u32`):
`REQUEST_BODY_PAUSED` and `REQUEST_BODY_BUFFER_ALL`.

## Verification

`bun bd test test/js/bun/http/serve.test.ts -t "request body
backpressure"`

Five tests next to the existing response-side backpressure tests: a
stalled streaming reader, a handler that defers touching `req.body`,
`Bun.write(file, req)` after the pre-stream pause, and `.arrayBuffer()`
with and without `req.body` touched first. Each writes a 32 MiB body
from a raw socket, polls the client's `sent` counter until it plateaus,
and asserts the plateau is well short of the total and the largest chunk
the handler sees is under 4 MiB. All five fail on `main` and pass with
this change; bytes are delivered intact.

The stale-`resp` paths (reader.read and req.text inside a direct-stream
`pull()` after `c.end()` across a loop tick) were verified under ASAN.
`node-http.test.ts` "pipelined responses buffered past the high water
mark pause reads on the connection" passes.

## Binary size

The +~530 KB flagged by the size check is measured against main build
79916, the last passing canary. Twelve commits landed on main between
that baseline and this PR's parent, including `node:quic` (#32602), the
full `node:repl` (#31827), and `node:inspector` (#31823), none of which
has produced a passing canary yet.

<!-- robobun:evidence:begin -->

---

**no test proof** · iteration 2 · Platform-specific test(s) that do not
run on this machine. Deferring to CI, which covers all platforms:
test/js/bun/http/serve.test.ts

<!-- robobun:evidence:end -->
social4hyq pushed a commit to social4hyq/ohos-bun that referenced this pull request Aug 2, 2026
…ms, class B)

expectations.txt (whole-file, they hang the runner):
- 07500 + 6 vendored test-repl files: T50 platform pipe-event loss
- readline.node.test.ts: T52 — the new v26 readline stack (oven-sh#31827) fails
  ~22 cursor-position assertions on OHOS; needs dedicated triage

case-level skipIf(isOHOS):
- node-http 'request via http proxy, issue#4295', node-tls-server
  SNICallback case: T49 (ADDRCONFIG localhost->::1) victims
- fs BigIntStats pre-epoch (same clamp as the already-quarantined sibling)
  and the 6 readdir-recursive x100 stress cases (chronic Node-mismatch +
  load-sensitive timeouts)
- mv cross-device describe: /dev/shm is not writable on OHOS (EACCES)
- process-stdin pipe-backpressure case: platform pipe coalescing (T50 family)

All modified files verified green on device with the 1.4.0_45 bottle. [skip ci]
social4hyq pushed a commit to social4hyq/ohos-bun that referenced this pull request Aug 2, 2026
…2 closed)

Root cause of the readline.node/ test-repl-* failures: agent shells export
TERM=dumb, which makes node:readline take its _ttyWriteDumb fallback where
cursor control keys are no-ops — every cursor-position assertion fails, and
the new v26 REPL tests (oven-sh#31827) fail alongside. Not a code bug: identical
behavior reproduced with harmonybrew node (V8), and the emitKeys generator
parses control bytes correctly on both runtimes.

Fix at the source of the environment: normalize TERM to xterm-256color in
the runner's spawnBun env and the harness bunEnv when it is 'dumb'. Tests
that exercise dumb-terminal behavior set TERM explicitly and are unaffected.

Reverts yesterday's over-attributed quarantines (6x test-repl + readline.node
were labeled T50; only 07500 is a genuine T50 pipe-event-loss hang). All 7
files verified green from a TERM=dumb shell. [skip ci]
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.

repl.start not implemented in Bun

5 participants