Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions lib/internal/readline/interface.js
Original file line number Diff line number Diff line change
Expand Up @@ -172,9 +172,11 @@ function InterfaceConstructor(input, output, completer, terminal) {
let crlfDelay;
let prompt = '> ';
let signal;
let inputOptions;

if (input?.input) {
// An options object was given
inputOptions = input;
output = input.output;
completer = input.completer;
terminal = input.terminal;
Expand Down Expand Up @@ -215,7 +217,6 @@ function InterfaceConstructor(input, output, completer, terminal) {
input.removeHistoryDuplicates = removeHistoryDuplicates;
}

this.setupHistoryManager(input);

if (completer !== undefined && typeof completer !== 'function') {
throw new ERR_INVALID_ARG_VALUE('completer', completer);
Expand All @@ -234,6 +235,7 @@ function InterfaceConstructor(input, output, completer, terminal) {
this[kSubstringSearch] = null;
this.output = output;
this.input = input;
this.setupHistoryManager(inputOptions || input);
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

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

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);

Copilot uses AI. Check for mistakes.
this[kUndoStack] = [];
this[kRedoStack] = [];
this[kPreviousCursorCols] = -1;
Expand Down Expand Up @@ -384,8 +386,19 @@ class Interface extends InterfaceConstructor {
setupHistoryManager(options) {
this.historyManager = new ReplHistory(this, options);

if (options.onHistoryFileLoaded) {
this.historyManager.initialize(options.onHistoryFileLoaded);
// 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);
Comment on lines +389 to +400
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.
}
}

ObjectDefineProperty(this, 'history', {
Expand Down
48 changes: 48 additions & 0 deletions test/parallel/test-readline-history-init-order.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
'use strict';

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

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.
const readline = require('readline');
const { PassThrough } = require('stream');

// Regression test for https://github.com/nodejs/node/issues/61526
// This test ensures that createInterface() doesn't crash when input
// has an onHistoryFileLoaded property (e.g., from a Proxy or jest.mock)

// Test case 1: options object with onHistoryFileLoaded as function
{
const input = new PassThrough();
input.onHistoryFileLoaded = () => {};

readline.createInterface({
input,
output: new PassThrough()
});
}

// Test case 2: options object without onHistoryFileLoaded
{
const input = new PassThrough();
readline.createInterface({
input,
output: new PassThrough()
});
}

// Test case 3: options object with onHistoryFileLoaded as non-function
{
const input = new PassThrough();
input.onHistoryFileLoaded = { some: 'object' };

readline.createInterface({
input,
output: new PassThrough()
});
}

// Test case 4: direct stream with onHistoryFileLoaded (original bug scenario)
{
const input = new PassThrough();
input.onHistoryFileLoaded = () => {};

readline.createInterface(input, new PassThrough());
}
4 changes: 3 additions & 1 deletion test/parallel/test-repl-persistent-history.js
Original file line number Diff line number Diff line change
Expand Up @@ -183,8 +183,10 @@ function cleanupTmpFile() {
}

// Copy our fixture to the tmp directory
const writeStream = fs.createWriteStream(historyPath);
fs.createReadStream(historyFixturePath)
.pipe(fs.createWriteStream(historyPath)).on('unpipe', () => runTest());
.pipe(writeStream);
writeStream.on('finish', () => runTest());

const runTestWrap = common.mustCall(runTest, numtests);

Expand Down
Loading