From 4c72492ae7e1f7cc962814b77e701207bd96528b Mon Sep 17 00:00:00 2001 From: margaretjgu Date: Tue, 12 May 2026 12:21:01 -0400 Subject: [PATCH 1/5] refactor: extract config cache into a dedicated module with clear/reset support Replaces the bare module-level `let earlyConfig` in cli.ts with a dedicated `src/config/cache.ts` module that exports getCachedConfig, setCachedConfig, and clearCachedConfig. This gives tests a safe reset hook (clearCachedConfig) to prevent stale state from leaking across test cases if the caching pattern spreads to library modules. Closes #210 --- src/cli.ts | 12 +++--- src/config/cache.ts | 31 ++++++++++++++++ test/config/cache.test.ts | 77 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 115 insertions(+), 5 deletions(-) create mode 100644 src/config/cache.ts create mode 100644 test/config/cache.test.ts diff --git a/src/cli.ts b/src/cli.ts index 106c03a8..af3ce95a 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -7,9 +7,10 @@ import { Command } from 'commander' import { defineCommand, defineGroup, hideBlockedCommands } from './factory.js' import type { OpaqueCommandHandle } from './factory.js' -import { loadConfig, type LoadConfigResult } from './config/loader.ts' +import { loadConfig } from './config/loader.ts' import { BUILT_IN_PROFILES, type BuiltInProfile } from './config/profiles.ts' import { setResolvedConfig } from './config/store.ts' +import { getCachedConfig, setCachedConfig } from './config/cache.ts' import { renderLogo } from './lib/logo.ts' // x-release-please-start-version @@ -34,8 +35,6 @@ program // // When no --config-file or --use-context overrides are specified, the hook // reuses the cached earlyConfig to avoid a redundant load+resolve cycle. -let earlyConfig: LoadConfigResult | undefined - program.hook('preAction', async (thisCommand, actionCommand) => { if (actionCommand.name() === 'version') return if (actionCommand.parent?.name() === 'docs') return @@ -48,6 +47,7 @@ program.hook('preAction', async (thisCommand, actionCommand) => { const { configFile: configPath, useContext: contextName, commandProfile: profileName } = thisCommand.opts() const typedProfileName = profileName as BuiltInProfile | undefined + const earlyConfig = getCachedConfig() if (configPath == null && contextName == null && profileName == null && earlyConfig?.ok === true) { setResolvedConfig(earlyConfig.value) return @@ -179,16 +179,17 @@ if (firstArg === 'sanitize') { // Load config early so --help can hide blocked commands. Skip for commands // that don't need config (e.g. `version`, `sanitize`, or `config` which authors the file) // to avoid unnecessary file I/O and a confusing "no config found" path. -// The result is cached in earlyConfig so the preAction hook can reuse it. +// The result is cached via setCachedConfig() so the preAction hook can reuse it. if (firstArg !== 'version' && firstArg !== 'config' && firstArg !== 'sanitize') { // Parse --profile early (before Commander's full parse) so the early config load // and hideBlockedCommands can apply the correct profile-based allow-list to --help. const profileArgIdx = process.argv.indexOf('--command-profile') const earlyProfile = profileArgIdx !== -1 ? process.argv[profileArgIdx + 1] as BuiltInProfile | undefined : undefined - earlyConfig = await loadConfig({ + const earlyConfig = await loadConfig({ ...(earlyProfile != null && { profileName: earlyProfile }), }) + setCachedConfig(earlyConfig) if (earlyConfig.ok) { setResolvedConfig(earlyConfig.value) hideBlockedCommands(program, earlyConfig.value.commands) @@ -196,6 +197,7 @@ if (firstArg !== 'version' && firstArg !== 'config' && firstArg !== 'sanitize') } if (process.argv.slice(2).length === 0) { + const earlyConfig = getCachedConfig() if (!earlyConfig?.ok || earlyConfig.value.banner !== false) { process.stdout.write(renderLogo(VERSION)) } diff --git a/src/config/cache.ts b/src/config/cache.ts new file mode 100644 index 00000000..6ea8b8f5 --- /dev/null +++ b/src/config/cache.ts @@ -0,0 +1,31 @@ +/* + * Copyright Elasticsearch B.V. and contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Cache for the early-loaded config result in cli.ts. + * + * Extracted from a bare module-level `let` in cli.ts into a dedicated module + * so that tests can call `clearCachedConfig()` to reset state between runs, + * preventing stale cached values from leaking across test cases. + */ + +import type { LoadConfigResult } from './loader.ts' + +let _cachedConfig: LoadConfigResult | undefined + +/** Returns the cached config result, or `undefined` if nothing has been cached yet. */ +export function getCachedConfig (): LoadConfigResult | undefined { + return _cachedConfig +} + +/** Stores a config load result in the cache. */ +export function setCachedConfig (result: LoadConfigResult): void { + _cachedConfig = result +} + +/** Clears the cached config. Intended for test cleanup only. */ +export function clearCachedConfig (): void { + _cachedConfig = undefined +} diff --git a/test/config/cache.test.ts b/test/config/cache.test.ts new file mode 100644 index 00000000..a6d89f11 --- /dev/null +++ b/test/config/cache.test.ts @@ -0,0 +1,77 @@ +/* + * Copyright Elasticsearch B.V. and contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, afterEach } from 'node:test' +import assert from 'node:assert/strict' +import { getCachedConfig, setCachedConfig, clearCachedConfig } from '../../src/config/cache.ts' +import type { LoadConfigResult } from '../../src/config/loader.ts' + +const okResult: LoadConfigResult = { + ok: true, + value: { + context: { + elasticsearch: { url: 'https://localhost:9200', auth: { api_key: 'key123' } }, + }, + }, +} + +const errResult: LoadConfigResult = { + ok: false, + error: { message: 'No configuration file found' }, +} + +afterEach(() => { + clearCachedConfig() +}) + +describe('config cache', () => { + describe('getCachedConfig', () => { + it('returns undefined before anything is cached', () => { + assert.equal(getCachedConfig(), undefined) + }) + }) + + describe('setCachedConfig / getCachedConfig', () => { + it('returns a cached ok result', () => { + setCachedConfig(okResult) + assert.deepEqual(getCachedConfig(), okResult) + }) + + it('returns a cached error result', () => { + setCachedConfig(errResult) + assert.deepEqual(getCachedConfig(), errResult) + }) + + it('returns the most recently cached result when called multiple times', () => { + setCachedConfig(errResult) + setCachedConfig(okResult) + assert.deepEqual(getCachedConfig(), okResult) + }) + + it('stores the reference, not a copy', () => { + setCachedConfig(okResult) + assert.equal(getCachedConfig(), okResult) + }) + }) + + describe('clearCachedConfig', () => { + it('resets the cache to undefined', () => { + setCachedConfig(okResult) + clearCachedConfig() + assert.equal(getCachedConfig(), undefined) + }) + + it('is safe to call multiple times', () => { + clearCachedConfig() + clearCachedConfig() + assert.equal(getCachedConfig(), undefined) + }) + + it('is safe to call before anything is cached', () => { + assert.doesNotThrow(() => clearCachedConfig()) + assert.equal(getCachedConfig(), undefined) + }) + }) +}) From 716152c701e44e7bd4960cfe3a9527afbc266a19 Mon Sep 17 00:00:00 2001 From: margaretjgu Date: Wed, 20 May 2026 13:48:02 -0400 Subject: [PATCH 2/5] refactor(config): move cache into loadConfig with refresh option Per JoshMock's suggestion: delete cache.ts and handle caching inside loadConfig itself via a refresh option (default false = use cache). - loadConfig() returns the cached result on repeat calls - loadConfig({ refresh: true }) bypasses cache and loads fresh - clearConfigCache() exported from loader.ts for test cleanup - cli.ts drops getCachedConfig/setCachedConfig imports; preAction hook passes refresh: true when CLI flags override the defaults - cache.test.ts rewired to test caching through loadConfig directly --- src/cli.ts | 18 +++---- src/config/cache.ts | 31 ------------ src/config/loader.ts | 28 +++++++++-- test/config/cache.test.ts | 103 +++++++++++++++++--------------------- 4 files changed, 76 insertions(+), 104 deletions(-) delete mode 100644 src/config/cache.ts diff --git a/src/cli.ts b/src/cli.ts index af3ce95a..435f9ebc 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -10,7 +10,6 @@ import type { OpaqueCommandHandle } from './factory.js' import { loadConfig } from './config/loader.ts' import { BUILT_IN_PROFILES, type BuiltInProfile } from './config/profiles.ts' import { setResolvedConfig } from './config/store.ts' -import { getCachedConfig, setCachedConfig } from './config/cache.ts' import { renderLogo } from './lib/logo.ts' // x-release-please-start-version @@ -33,8 +32,8 @@ program // On error, print a structured message and exit -- never let a config failure // silently propagate into the command handler. // -// When no --config-file or --use-context overrides are specified, the hook -// reuses the cached earlyConfig to avoid a redundant load+resolve cycle. +// When no --config-file or --use-context overrides are specified, loadConfig() +// returns the in-process cached result from the early load, avoiding redundant I/O. program.hook('preAction', async (thisCommand, actionCommand) => { if (actionCommand.name() === 'version') return if (actionCommand.parent?.name() === 'docs') return @@ -46,17 +45,13 @@ program.hook('preAction', async (thisCommand, actionCommand) => { } const { configFile: configPath, useContext: contextName, commandProfile: profileName } = thisCommand.opts() const typedProfileName = profileName as BuiltInProfile | undefined - - const earlyConfig = getCachedConfig() - if (configPath == null && contextName == null && profileName == null && earlyConfig?.ok === true) { - setResolvedConfig(earlyConfig.value) - return - } + const hasOverrides = configPath != null || contextName != null || profileName != null const result = await loadConfig({ ...(configPath != null && { configPath }), ...(contextName != null && { contextName }), ...(typedProfileName != null && { profileName: typedProfileName }), + refresh: hasOverrides, }) if (result.ok) { setResolvedConfig(result.value) @@ -179,7 +174,7 @@ if (firstArg === 'sanitize') { // Load config early so --help can hide blocked commands. Skip for commands // that don't need config (e.g. `version`, `sanitize`, or `config` which authors the file) // to avoid unnecessary file I/O and a confusing "no config found" path. -// The result is cached via setCachedConfig() so the preAction hook can reuse it. +// loadConfig() caches the result in-process; the preAction hook reuses it via the default cache path. if (firstArg !== 'version' && firstArg !== 'config' && firstArg !== 'sanitize') { // Parse --profile early (before Commander's full parse) so the early config load // and hideBlockedCommands can apply the correct profile-based allow-list to --help. @@ -189,7 +184,6 @@ if (firstArg !== 'version' && firstArg !== 'config' && firstArg !== 'sanitize') const earlyConfig = await loadConfig({ ...(earlyProfile != null && { profileName: earlyProfile }), }) - setCachedConfig(earlyConfig) if (earlyConfig.ok) { setResolvedConfig(earlyConfig.value) hideBlockedCommands(program, earlyConfig.value.commands) @@ -197,7 +191,7 @@ if (firstArg !== 'version' && firstArg !== 'config' && firstArg !== 'sanitize') } if (process.argv.slice(2).length === 0) { - const earlyConfig = getCachedConfig() + const earlyConfig = await loadConfig() if (!earlyConfig?.ok || earlyConfig.value.banner !== false) { process.stdout.write(renderLogo(VERSION)) } diff --git a/src/config/cache.ts b/src/config/cache.ts deleted file mode 100644 index 6ea8b8f5..00000000 --- a/src/config/cache.ts +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -/** - * Cache for the early-loaded config result in cli.ts. - * - * Extracted from a bare module-level `let` in cli.ts into a dedicated module - * so that tests can call `clearCachedConfig()` to reset state between runs, - * preventing stale cached values from leaking across test cases. - */ - -import type { LoadConfigResult } from './loader.ts' - -let _cachedConfig: LoadConfigResult | undefined - -/** Returns the cached config result, or `undefined` if nothing has been cached yet. */ -export function getCachedConfig (): LoadConfigResult | undefined { - return _cachedConfig -} - -/** Stores a config load result in the cache. */ -export function setCachedConfig (result: LoadConfigResult): void { - _cachedConfig = result -} - -/** Clears the cached config. Intended for test cleanup only. */ -export function clearCachedConfig (): void { - _cachedConfig = undefined -} diff --git a/src/config/loader.ts b/src/config/loader.ts index fea96835..7477d00e 100644 --- a/src/config/loader.ts +++ b/src/config/loader.ts @@ -219,6 +219,23 @@ export interface LoadConfigOptions { contextName?: string /** Profile name override (`--profile` flag). Overrides any profile set in the config file. */ profileName?: BuiltInProfile + /** + * When `true`, bypass the in-process cache and perform a fresh load (caching + * the new result for subsequent calls). Defaults to `false`. + */ + refresh?: boolean +} + +// --------------------------------------------------------------------------- +// In-process result cache — avoids redundant I/O within a single CLI invocation. +// Tests can call clearConfigCache() between cases to prevent state leakage. +// --------------------------------------------------------------------------- + +let _cachedConfig: LoadConfigResult | undefined + +/** Clears the in-process config cache. Intended for test cleanup only. */ +export function clearConfigCache (): void { + _cachedConfig = undefined } /** Successful result from {@link loadConfig}. */ @@ -251,7 +268,9 @@ export type LoadConfigResult = LoadConfigOk | LoadConfigErr * @returns A `LoadConfigResult` discriminated union. */ export async function loadConfig (options: LoadConfigOptions = {}): Promise { - const { configPath, contextName, profileName } = options + const { configPath, contextName, profileName, refresh = false } = options + + if (!refresh && _cachedConfig !== undefined) return _cachedConfig // Validate profileName early (before any I/O) so the error is immediate and clear if (profileName != null && !(BUILT_IN_PROFILES as readonly string[]).includes(profileName)) { @@ -358,10 +377,13 @@ export async function loadConfig (options: LoadConfigOptions = {}): Promise { + let tmpDir: string + let configPath: string -afterEach(() => { - clearCachedConfig() -}) - -describe('config cache', () => { - describe('getCachedConfig', () => { - it('returns undefined before anything is cached', () => { - assert.equal(getCachedConfig(), undefined) - }) + before(async () => { + tmpDir = await mkdtemp(join(tmpdir(), 'elastic-cli-cache-test-')) + configPath = join(tmpDir, '.elasticrc.yml') + await writeFile(configPath, VALID_CONFIG_YAML, 'utf-8') }) - describe('setCachedConfig / getCachedConfig', () => { - it('returns a cached ok result', () => { - setCachedConfig(okResult) - assert.deepEqual(getCachedConfig(), okResult) - }) - - it('returns a cached error result', () => { - setCachedConfig(errResult) - assert.deepEqual(getCachedConfig(), errResult) - }) + after(async () => rm(tmpDir, { recursive: true })) - it('returns the most recently cached result when called multiple times', () => { - setCachedConfig(errResult) - setCachedConfig(okResult) - assert.deepEqual(getCachedConfig(), okResult) - }) + afterEach(() => clearConfigCache()) - it('stores the reference, not a copy', () => { - setCachedConfig(okResult) - assert.equal(getCachedConfig(), okResult) - }) + it('returns the same reference on repeated calls (cache hit)', async () => { + const first = await loadConfig({ configPath }) + const second = await loadConfig({ configPath }) + assert.equal(first, second, 'expected same object reference from cache') }) - describe('clearCachedConfig', () => { - it('resets the cache to undefined', () => { - setCachedConfig(okResult) - clearCachedConfig() - assert.equal(getCachedConfig(), undefined) - }) + it('returns a fresh result when refresh: true', async () => { + const first = await loadConfig({ configPath }) + const second = await loadConfig({ configPath, refresh: true }) + assert.notEqual(first, second, 'expected a new object when refresh is true') + assert.deepEqual(first, second, 'expected same shape after refresh') + }) - it('is safe to call multiple times', () => { - clearCachedConfig() - clearCachedConfig() - assert.equal(getCachedConfig(), undefined) - }) + it('clearConfigCache resets to undefined so next call loads fresh', async () => { + const first = await loadConfig({ configPath }) + clearConfigCache() + const second = await loadConfig({ configPath }) + assert.notEqual(first, second, 'expected a new object after cache was cleared') + assert.deepEqual(first, second, 'expected same shape after re-load') + }) - it('is safe to call before anything is cached', () => { - assert.doesNotThrow(() => clearCachedConfig()) - assert.equal(getCachedConfig(), undefined) - }) + it('refresh: true updates the cache so subsequent default calls return the refreshed result', async () => { + await loadConfig({ configPath }) + const refreshed = await loadConfig({ configPath, refresh: true }) + const subsequent = await loadConfig({ configPath }) + assert.equal(refreshed, subsequent, 'expected the refreshed result to be cached') }) }) From 62cf619aea5d4879428d0eb24e58bc81a4d3241a Mon Sep 17 00:00:00 2001 From: margaretjgu Date: Thu, 21 May 2026 10:13:25 -0400 Subject: [PATCH 3/5] fix(test): clear config cache in afterEach to prevent cross-test bleed --- packages/config-resolver/test/index.test.ts | 3 ++- test/config/loader.test.ts | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/config-resolver/test/index.test.ts b/packages/config-resolver/test/index.test.ts index 37d0b72d..5f3536af 100644 --- a/packages/config-resolver/test/index.test.ts +++ b/packages/config-resolver/test/index.test.ts @@ -8,7 +8,7 @@ import assert from 'node:assert/strict' import { mkdtemp, writeFile, rm } from 'node:fs/promises' import { join } from 'node:path' import { tmpdir } from 'node:os' -import { loadConfig } from '../../../src/config/loader.ts' +import { loadConfig, clearConfigCache } from '../../../src/config/loader.ts' import { containsExpression, resolveString, @@ -21,6 +21,7 @@ import { afterEach(() => { _testResetResolvers() + clearConfigCache() }) // --------------------------------------------------------------------------- diff --git a/test/config/loader.test.ts b/test/config/loader.test.ts index 583a38a3..cbcac0fe 100644 --- a/test/config/loader.test.ts +++ b/test/config/loader.test.ts @@ -3,14 +3,16 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { describe, it, before, after } from 'node:test' +import { describe, it, before, after, afterEach } from 'node:test' import assert from 'node:assert/strict' import { mkdtemp, writeFile, rm, mkdir } from 'node:fs/promises' import { join } from 'node:path' import { tmpdir } from 'node:os' -import { loadConfigFile, discoverConfigFile, resolveContext, resolveEffectiveCommands, loadConfig } from '../../src/config/loader.ts' +import { loadConfigFile, discoverConfigFile, resolveContext, resolveEffectiveCommands, loadConfig, clearConfigCache } from '../../src/config/loader.ts' import type { ConfigFile, ResolvedConfig } from '../../src/config/types.ts' +afterEach(() => clearConfigCache()) + // --------------------------------------------------------------------------- // Shared fixtures // --------------------------------------------------------------------------- From 2088c096238f436158cc7bc059d28ad6ae4117ba Mon Sep 17 00:00:00 2001 From: margaretjgu Date: Thu, 21 May 2026 10:18:57 -0400 Subject: [PATCH 4/5] chore: update NOTICE.txt for yaml and zod version bumps --- NOTICE.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/NOTICE.txt b/NOTICE.txt index 5f32f5a7..7e28dce8 100644 --- a/NOTICE.txt +++ b/NOTICE.txt @@ -1133,7 +1133,7 @@ THIS SOFTWARE. ------------------------------------------------------------------------ -yaml@2.8.3 +yaml@2.8.4 License: ISC Repository: https://github.com/eemeli/yaml Publisher: Eemeli Aro @@ -1207,7 +1207,7 @@ THE SOFTWARE. ------------------------------------------------------------------------ -zod@4.3.6 +zod@4.4.3 License: MIT Repository: https://github.com/colinhacks/zod Publisher: Colin McDonnell From 7ae7ba910fc3a07edc71e4028bc591cddfdb03bb Mon Sep 17 00:00:00 2001 From: margaretjgu Date: Thu, 21 May 2026 10:20:07 -0400 Subject: [PATCH 5/5] fix(test): clear config cache in status afterEach --- test/status/register.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/status/register.test.ts b/test/status/register.test.ts index 83abc1e2..9a7f25e4 100644 --- a/test/status/register.test.ts +++ b/test/status/register.test.ts @@ -10,6 +10,7 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import { Command } from 'commander' import { registerStatusCommand, runStatusChecks, _testSetFetch } from '../../src/status/register.ts' +import { clearConfigCache } from '../../src/config/loader.ts' import type { ResolvedContext } from '../../src/config/types.ts' const SAMPLE_HEALTH = JSON.stringify({ status: 'green', number_of_nodes: 3 }) @@ -113,6 +114,7 @@ describe('elastic status -- command', () => { afterEach(async () => { await rm(dir, { recursive: true, force: true }) + clearConfigCache() }) async function writeConfig (yaml: string): Promise {