fix(run): stop double-wrapping startup URLs when node.hostname is a URL

bin/run.ts

@@ -25,7 +25,7 @@ import * as keys from '../security/keys.ts';
 import { startHTTPThreads } from '../server/threads/socketRouter.ts';
 import * as hdbInfoController from '../dataLayer/hdbInfoController.ts';
 import { isReadOnlyMode } from '../resources/databases.ts';
-import { getThisNodeName } from '../server/nodeName.ts';
+import { getThisNodeName, getThisNodeHostname } from '../server/nodeName.ts';
 import * as hdbTerms from '../utility/hdbTerms.ts';
 import { getHdbPid, isProcessRunning } from '../utility/processManagement/processManagement.js';
 import { PACKAGE_ROOT } from '../utility/packageUtils.js';
@@ -321,17 +321,10 @@ function startupLog(portResolutions: any) {
 	}`;
 	logMsg += `, unix socket: ${configUtils.getConfigPath(CONFIG_PARAMS.OPERATIONSAPI_NETWORK_DOMAINSOCKET)}\n`;
 	if (env.get(CONFIG_PARAMS.OPERATIONSAPI_NETWORK_PORT)) {
-		logMsg += pad('') + 'http://' + getThisNodeName() + ':' + env.get(CONFIG_PARAMS.OPERATIONSAPI_NETWORK_PORT) + '/\n';
+		logMsg += `${pad('')}http://${getThisNodeHostname()}:${env.get(CONFIG_PARAMS.OPERATIONSAPI_NETWORK_PORT)}/\n`;
 	}
 	if (env.get(CONFIG_PARAMS.OPERATIONSAPI_NETWORK_SECUREPORT)) {
-		logMsg +=
-			'\n' +
-			pad('') +
-			'https://' +
-			getThisNodeName() +
-			':' +
-			env.get(CONFIG_PARAMS.OPERATIONSAPI_NETWORK_SECUREPORT) +
-			'/\n';
+		logMsg += `\n${pad('')}https://${getThisNodeHostname()}:${env.get(CONFIG_PARAMS.OPERATIONSAPI_NETWORK_SECUREPORT)}/\n`;
 	}
 
 	// MQTT Log
@@ -383,7 +376,7 @@ function startupLog(portResolutions: any) {
 			if (!restLog.includes(pair) && name === 'rest') {
 				restLog += pair;
 				if (value.protocol_name === 'HTTP' || value.protocol_name === 'HTTPS') {
-					restHostnames.push(`${value.protocol_name.toLowerCase()}://${getThisNodeName()}:${key}/`);
+					restHostnames.push(`${value.protocol_name.toLowerCase()}://${getThisNodeHostname()}:${key}/`);
 				}
 			}
 

server/nodeName.ts

@@ -58,6 +58,25 @@ export function clearThisNodeName() {
 	nodeName = undefined;
 }
 
+// node.hostname may be configured as a full URL; reduce it to a bare host so composing a
+// display URL from it does not double-wrap the scheme and port.
+export function nodeNameToDisplayHost(name: string): string {
+	if (!name) return name;
+	const hasScheme = /^[a-z][a-z0-9+.-]*:\/\//i.test(name);
+	const candidates = hasScheme ? [name] : [`http://${name}`, `http://[${name}]`];
+	for (const candidate of candidates) {
+		try {
+			const { hostname } = new URL(candidate);
+			if (hostname) return hostname;
+		} catch {}
+	}
+	return name;
+}
+
+export function getThisNodeHostname(): string {
+	return nodeNameToDisplayHost(getThisNodeName());
+}
+
 function getHostFromListeningPort(key: string) {
 	const port: string | undefined = env.get(key);
 	const lastColon = port?.lastIndexOf?.(':');

unitTests/bin/startupBanner.test.js

@@ -0,0 +1,116 @@
+'use strict';
+
+// End-to-end guard for the #2218 fix (double-wrapped startup URLs). The pure helper
+// nodeNameToDisplayHost() is unit-tested in unitTests/server/nodeName.test.js, but nothing exercised
+// startupLog() itself, so a call site that switched back from getThisNodeHostname() to
+// getThisNodeName() would recreate the `http://http://host:port/` banner while that suite stayed
+// green. This drives the real, exported startupLog() with a real (URL-valued) node.hostname and
+// asserts the Operations-API HTTP/HTTPS and REST URL lines it prints are well-formed.
+
+const testUtils = require('../testUtils.js');
+testUtils.preTestPrep();
+
+const assert = require('assert');
+
+const env = require('#src/utility/environment/environmentManager');
+const { CONFIG_PARAMS } = require('#src/utility/hdbTerms');
+const { clearThisNodeName } = require('#src/server/nodeName');
+const { startupLog } = require('#src/bin/run');
+
+describe('startupLog banner URLs (#2218 double-wrapped startup URLs)', () => {
+	const originalConsoleLog = console.log;
+	// initTestEnvironment() sets operationsApi_network_port=9925 and http_port=9926; the secure
+	// operations port is unset by default, so tests that need it set (and restore) it explicitly. Read
+	// the live ops port at assert time rather than caching it, so the expected URL always tracks the
+	// same config value startupLog() reads (no load-time-vs-run-time skew if another suite mutates it).
+	const opsPort = () => env.get(CONFIG_PARAMS.OPERATIONSAPI_NETWORK_PORT);
+
+	let originalNodeHostname;
+	let originalSecurePort;
+
+	beforeEach(() => {
+		originalNodeHostname = env.get(CONFIG_PARAMS.NODE_HOSTNAME);
+		originalSecurePort = env.get(CONFIG_PARAMS.OPERATIONSAPI_NETWORK_SECUREPORT);
+	});
+
+	afterEach(() => {
+		console.log = originalConsoleLog;
+		// Restore (not blank) the config this suite mutated so it can't leak into other suites in
+		// the same mocha process.
+		env.setProperty(CONFIG_PARAMS.NODE_HOSTNAME, originalNodeHostname);
+		env.setProperty(CONFIG_PARAMS.OPERATIONSAPI_NETWORK_SECUREPORT, originalSecurePort);
+		clearThisNodeName();
+	});
+
+	// Runs startupLog() with node.hostname set to `hostname`, captures the emitted banner, and
+	// returns it as a single string.
+	function bannerFor(hostname, portResolutions = new Map()) {
+		env.setProperty(CONFIG_PARAMS.NODE_HOSTNAME, hostname);
+		clearThisNodeName(); // getThisNodeName() memoizes; drop any value cached by an earlier test
+
+		const lines = [];
+		console.log = (...args) => lines.push(args.join(' '));
+		try {
+			startupLog(portResolutions);
+		} finally {
+			console.log = originalConsoleLog;
+		}
+		return lines.join('\n');
+	}
+
+	it('composes well-formed ops + REST URLs when node.hostname is a full URL', () => {
+		// A URL-valued node.hostname is exactly the #2218 trigger: getThisNodeName() returns the whole
+		// URL, so the pre-fix banner produced `http://http://localhost:9926:9925/`.
+		const restPort = 9926;
+		const portResolutions = new Map([[restPort, [{ name: 'rest', protocol_name: 'HTTP' }]]]);
+
+		const banner = bannerFor('http://localhost:9926', portResolutions);
+
+		assert.ok(!banner.includes('http://http://'), `banner double-wrapped a scheme:\n${banner}`);
+		assert.ok(
+			banner.includes(`http://localhost:${opsPort()}/`),
+			`expected the Operations-API URL http://localhost:${opsPort()}/ in banner:\n${banner}`
+		);
+		assert.ok(
+			banner.includes(`http://localhost:${restPort}/`),
+			`expected the REST URL http://localhost:${restPort}/ in banner:\n${banner}`
+		);
+	});
+
+	it('composes well-formed HTTPS ops + REST URLs when node.hostname is a full URL', () => {
+		const securePort = 9935;
+		const restPort = 9927;
+		env.setProperty(CONFIG_PARAMS.OPERATIONSAPI_NETWORK_SECUREPORT, securePort);
+		const portResolutions = new Map([[restPort, [{ name: 'rest', protocol_name: 'HTTPS' }]]]);
+
+		const banner = bannerFor('https://localhost:9926', portResolutions);
+
+		assert.ok(!banner.includes('https://https://'), `banner double-wrapped a scheme:\n${banner}`);
+		assert.ok(
+			banner.includes(`https://localhost:${securePort}/`),
+			`expected the Operations-API URL https://localhost:${securePort}/ in banner:\n${banner}`
+		);
+		assert.ok(
+			banner.includes(`https://localhost:${restPort}/`),
+			`expected the REST URL https://localhost:${restPort}/ in banner:\n${banner}`
+		);
+	});
+
+	it('leaves a plain bare-host node.hostname unchanged in composed URLs', () => {
+		// The complement of the URL case: normalization must not corrupt a hostname that was already
+		// bare (over-stripping would be just as wrong as double-wrapping).
+		const restPort = 9926;
+		const portResolutions = new Map([[restPort, [{ name: 'rest', protocol_name: 'HTTP' }]]]);
+
+		const banner = bannerFor('node-1.example.com', portResolutions);
+
+		assert.ok(
+			banner.includes(`http://node-1.example.com:${opsPort()}/`),
+			`expected the Operations-API URL http://node-1.example.com:${opsPort()}/ in banner:\n${banner}`
+		);
+		assert.ok(
+			banner.includes(`http://node-1.example.com:${restPort}/`),
+			`expected the REST URL http://node-1.example.com:${restPort}/ in banner:\n${banner}`
+		);
+	});
+});

unitTests/server/nodeName.test.js

@@ -8,7 +8,13 @@ const sinon = require('sinon');
 
 const env = require('#src/utility/environment/environmentManager');
 const { logger } = require('#src/utility/logging/logger');
-const { hostnameToUrl, getThisNodeName, clearThisNodeName } = require('#src/server/nodeName');
+const {
+	hostnameToUrl,
+	getThisNodeName,
+	getThisNodeHostname,
+	nodeNameToDisplayHost,
+	clearThisNodeName,
+} = require('#src/server/nodeName');
 
 describe('getThisNodeName precedence (harper-pro#351)', () => {
 	let sandbox;
@@ -78,6 +84,54 @@ describe('getThisNodeName precedence (harper-pro#351)', () => {
 	});
 });
 
+describe('nodeNameToDisplayHost (#2218 double-wrapped startup URLs)', () => {
+	it('returns a bare host unchanged', () => {
+		assert.strictEqual(nodeNameToDisplayHost('localhost'), 'localhost');
+	});
+
+	it('strips the scheme and port when node.hostname is a full URL', () => {
+		assert.strictEqual(nodeNameToDisplayHost('http://localhost:9926'), 'localhost');
+	});
+
+	it('strips a bare host:port down to the host', () => {
+		assert.strictEqual(nodeNameToDisplayHost('localhost:9926'), 'localhost');
+	});
+
+	it('preserves an already-bracketed IPv6 host', () => {
+		assert.strictEqual(nodeNameToDisplayHost('https://[::1]:9926'), '[::1]');
+	});
+
+	it('brackets a bare IPv6 literal so composed URLs stay valid', () => {
+		assert.strictEqual(nodeNameToDisplayHost('::1'), '[::1]');
+	});
+
+	it('returns an unparseable value unchanged rather than dropping it', () => {
+		assert.strictEqual(nodeNameToDisplayHost('node with space'), 'node with space');
+	});
+});
+
+describe('getThisNodeHostname reads and normalizes the configured node.hostname', () => {
+	let originalNodeHostname;
+
+	beforeEach(() => {
+		originalNodeHostname = env.get('node_hostname');
+		clearThisNodeName();
+	});
+
+	afterEach(() => {
+		env.setProperty('node_hostname', originalNodeHostname);
+		clearThisNodeName();
+	});
+
+	// Guards the wiring bin/run.ts depends on: the wrapper must normalize the resolved node name,
+	// not return it raw.
+	it('normalizes a URL-valued node.hostname to a bare host', () => {
+		env.setProperty('node_hostname', 'http://localhost:9926');
+		clearThisNodeName();
+		assert.strictEqual(getThisNodeHostname(), 'localhost');
+	});
+});
+
 describe('hostnameToUrl', () => {
 	let sandbox;