readline: initialize input before history manager #61538

Zohaibarif69 · 2026-01-27T01:24:55Z

Fixes a regression where setupHistoryManager() could run before
this.input was assigned, causing a crash when onHistoryFileLoaded
is present (for example, Proxy or jest.mock() inputs).

This change ensures this.input is initialized before history setup
and adds a regression test to cover the scenario.

Fixes: #61526

Testing:

Added regression test
Unable to run tests locally due to Windows build issues; CI should validate

lib/internal/readline/interface.js

thisalihassan · 2026-01-28T11:09:58Z

lib/internal/readline/interface.js

  this[kSubstringSearch] = null;
  this.output = output;
  this.input = input;
+  this.setupHistoryManager(input);


this reodering only resolve this.input being undefine but still doesn't fix accidental REPL history init where readline.createInterface(input) is not supposed to touch REPL history but setupHistoryManager(input) ends up seeing a truthy input.onHistoryFileLoaded and calls ReplHistory.initialize() anyway

test/parallel/test-readline-history-init-order.js

Zohaibarif69 · 2026-01-28T18:30:44Z

@thisalihassan Thanks for the feedback! I’ll address the scoping, test, and history initialization issues and update the PR shortly.

Zohaibarif69 · 2026-01-29T05:41:26Z

@thisalihassan Thanks — I’ve addressed the scoping, history initialization, and test issues you mentioned.
Would appreciate a re-review.

Zohaibarif69 · 2026-01-29T09:59:44Z

@thisalihassan I’ve cleaned up the diff, reverted the unnecessary scoping changes, fixed the history initialization logic, and updated the test to use real streams.
Would appreciate a re-review when you have time.

thisalihassan

overall changes lg but we can improve tests I believe. I will wait someone from the node team to request CI and review the PR

Zohaibarif69 · 2026-02-06T05:28:09Z

Hi Node.js maintainers
Review feedback has been addressed and the PR is ready.
Would appreciate CI approval and a review when possible.
Thanks!

codecov · 2026-02-11T03:44:07Z

Codecov Report

❌ Patch coverage is 62.50000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.43%. Comparing base (68d7b6f) to head (9dc21a9).
⚠️ Report is 112 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/internal/readline/interface.js	62.50%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #61538      +/-   ##
==========================================
- Coverage   89.77%   89.43%   -0.35%     
==========================================
  Files         673      673              
  Lines      203820   203815       -5     
  Branches    39175    38998     -177     
==========================================
- Hits       182987   182275     -712     
- Misses      13152    13837     +685     
- Partials     7681     7703      +22

Files with missing lines	Coverage Δ
lib/internal/readline/interface.js	`89.83% <62.50%> (-7.51%)`	⬇️

... and 46 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…dling - Reorder initialization: assign this.input before setupHistoryManager() - Preserve original options object for proper history manager initialization - Fix REPL.setupHistory() callback invocation logic - Fix stream event handling in repl persistent history test Fixes: nodejs#61526

Copilot

Pull request overview

Fixes a readline regression where history initialization could run before this.input is assigned (triggered by an onHistoryFileLoaded property on the input), and adds tests to prevent reintroducing the crash.

Changes:

Move setupHistoryManager() to run after this.input is assigned and add guards to avoid initializing persistent history from the readline constructor path.
Add a regression test covering inputs with an onHistoryFileLoaded property across createInterface() call patterns.
Adjust the REPL persistent history test to wait for the history fixture copy to complete before running the suite.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
lib/internal/readline/interface.js	Reorders history manager setup and changes initialization conditions for persistent history loading.
test/parallel/test-readline-history-init-order.js	Adds regression coverage for inputs that expose `onHistoryFileLoaded`.
test/parallel/test-repl-persistent-history.js	Updates fixture copy sequencing before REPL history tests run.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-11T20:11:45Z

lib/internal/readline/interface.js

+    // Only initialize REPL history when called from REPL.setupHistory(),
+    // not from the readline constructor. 
+    // Constructor passes: stream OR { input: stream, output: stream, ... }
+    // setupHistory passes: { filePath: ..., size: ..., onHistoryFileLoaded: ... }
+    // Detect constructor calls by checking for stream.on() or options.input
+    if (options && typeof options === 'object') {
+      const isStream = typeof options.on === 'function';
+      const hasInputProperty = options.input !== undefined;
+      const isFromConstructor = isStream || hasInputProperty;
+
+      if (!isFromConstructor && typeof options.onHistoryFileLoaded === 'function') {
+        this.historyManager.initialize(options.onHistoryFileLoaded);


There are trailing whitespace characters in this comment block/blank line (e.g. after constructor. and on the blank line before the if), which will typically fail the repo's JS linting (no-trailing-spaces). Please remove trailing spaces.

Copilot · 2026-02-11T20:11:45Z

test/parallel/test-readline-history-init-order.js

@@ -0,0 +1,48 @@
+'use strict';
+
+const common = require('../common');


common is required but never used in this test file. This will fail linting with no-unused-vars in the test suite. Either remove the import or use common (e.g., common.mustNotCall, common.mustCall, or common.skip(...) if needed).

Suggested change

const common = require('../common');

Copilot · 2026-02-11T20:11:46Z

lib/internal/readline/interface.js

  this[kSubstringSearch] = null;
  this.output = output;
  this.input = input;
+  this.setupHistoryManager(inputOptions || input);


inputOptions (the original options object) is passed into setupHistoryManager(), but ReplHistory only reads options.size/options.history (not historySize). In the options-object path you currently only copy historySize into input.size, so passing inputOptions here will ignore historySize and can break existing readline.createInterface({ historySize }) behavior/tests. Pass the actual input stream (with .size/.history applied), or also copy size/history/removeHistoryDuplicates onto inputOptions before calling setupHistoryManager().

Suggested change

this.setupHistoryManager(inputOptions || input);

const historyOptions = inputOptions || input;

if (inputOptions !== undefined && input !== undefined) {

if (input.size !== undefined && historyOptions.size === undefined)

historyOptions.size = input.size;

if (input.history !== undefined && historyOptions.history === undefined)

historyOptions.history = input.history;

if (input.removeHistoryDuplicates !== undefined &&

historyOptions.removeHistoryDuplicates === undefined) {

historyOptions.removeHistoryDuplicates = input.removeHistoryDuplicates;

}

}

this.setupHistoryManager(historyOptions);

nodejs-github-bot added needs-ci PRs that need a full CI run. readline Issues and PRs related to the built-in readline module. labels Jan 27, 2026

thisalihassan reviewed Jan 28, 2026

View reviewed changes

Zohaibarif69 force-pushed the fix-readline-history-init branch from 736d855 to e3a00ce Compare January 29, 2026 00:19

readline: initialize input before history manager

9dc21a9

Zohaibarif69 force-pushed the fix-readline-history-init branch from e3a00ce to 9dc21a9 Compare January 29, 2026 09:46

thisalihassan reviewed Jan 30, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings February 11, 2026 20:02

Copilot started reviewing on behalf of Zohaibarif69 February 11, 2026 20:03 View session

Copilot AI reviewed Feb 11, 2026

View reviewed changes

		@@ -0,0 +1,48 @@
		'use strict';

		const common = require('../common');

-  this.setupHistoryManager(inputOptions || input);
+  const historyOptions = inputOptions || input;
+  if (inputOptions !== undefined && input !== undefined) {
+    if (input.size !== undefined && historyOptions.size === undefined)
+      historyOptions.size = input.size;
+    if (input.history !== undefined && historyOptions.history === undefined)
+      historyOptions.history = input.history;
+    if (input.removeHistoryDuplicates !== undefined &&
+        historyOptions.removeHistoryDuplicates === undefined) {
+      historyOptions.removeHistoryDuplicates = input.removeHistoryDuplicates;
+    }
+  }
+  this.setupHistoryManager(historyOptions);

Uh oh!

readline: initialize input before history manager #61538

Are you sure you want to change the base?

readline: initialize input before history manager #61538

Conversation

Zohaibarif69 commented Jan 27, 2026 • edited by Renegade334 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thisalihassan Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Zohaibarif69 commented Jan 28, 2026

Uh oh!

Zohaibarif69 commented Jan 29, 2026

Uh oh!

Zohaibarif69 commented Jan 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thisalihassan left a comment

Choose a reason for hiding this comment

Uh oh!

Zohaibarif69 commented Feb 6, 2026

Uh oh!

codecov bot commented Feb 11, 2026

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Zohaibarif69 commented Jan 27, 2026 •

edited by Renegade334

Loading

Zohaibarif69 commented Jan 29, 2026 •

edited

Loading