diff --git a/packages/keyman/src/keyman.args.ts b/packages/keyman/src/keyman.args.ts index 5595a81..5dfd4ba 100644 --- a/packages/keyman/src/keyman.args.ts +++ b/packages/keyman/src/keyman.args.ts @@ -179,7 +179,8 @@ Usage Flags -h, --help print this help and exit -V, --version print the version and exit - --print-config print the resolved vault paths as JSON and exit + --print-config print the resolved paths and the config files + they came from, as JSON, and exit --self-update same as the self-update subcommand Flags for self-update diff --git a/packages/keyman/src/keyman.cli.ts b/packages/keyman/src/keyman.cli.ts index f86a975..9cf068c 100644 --- a/packages/keyman/src/keyman.cli.ts +++ b/packages/keyman/src/keyman.cli.ts @@ -2,7 +2,7 @@ import { createRequire } from 'node:module'; import { helpText, type ParsedArgs, parseArgs, UsageError } from './keyman.args.js'; -import { loadConfig, resolveConfigPaths } from './keyman.config.js'; +import { describeConfig } from './keyman.config.js'; import { keyman } from './keyman.main.js'; import { formatCommand, selfUpdate, updateNotice } from './keyman.update.js'; @@ -34,9 +34,7 @@ if (parsed.command === 'help') { } if (parsed.command === 'print-config') { - const config = loadConfig(); - const paths = resolveConfigPaths(config); - console.log(JSON.stringify(paths)); + console.log(JSON.stringify(describeConfig())); process.exit(0); } diff --git a/packages/keyman/src/keyman.config.ts b/packages/keyman/src/keyman.config.ts index c55c9e3..07a59a6 100644 --- a/packages/keyman/src/keyman.config.ts +++ b/packages/keyman/src/keyman.config.ts @@ -15,27 +15,11 @@ const KeymanConfigSchema = z.object({ export type KeymanConfig = z.infer; -/** - * Resolution strategy for merging config properties - * - 'merge': Arrays are concatenated, objects are deep merged (default) - * - 'override': Child value completely replaces parent value - */ -export type ResolutionStrategy = 'merge' | 'override'; +/** Raw config file structure */ +export type KeymanConfigFile = Partial; -/** - * Resolution configuration for customizing merge behavior - */ -export type KeymanResolutionConfig = { - [K in keyof KeymanConfig]?: ResolutionStrategy; -}; - -/** - * Raw config file structure (includes resolution) - */ -export interface KeymanConfigFile extends Partial { - /** Customize merge behavior for specific properties */ - resolution?: KeymanResolutionConfig; -} +/** Every key a config file may set. */ +const KNOWN_KEYS = Object.keys(KeymanConfigSchema.shape) as (keyof KeymanConfig)[]; /** * Default configuration values @@ -110,71 +94,36 @@ function findConfigFiles(startDir: string): string[] { } /** - * Deep merges two values based on resolution strategy + * Reports keys a config file sets that keyman does not read. + * + * `z.object` strips them silently, so `{"vaultroot": "…"}` used to be + * indistinguishable from an empty file — the vault quietly stayed at the default + * and nothing said why. Warned rather than fatal, which is this module's posture + * throughout, and warned *here* because this is the only place the filename is in + * hand: `z.strictObject` on the merged result cannot name the file that said it. */ -function mergeValue( - parentValue: unknown, - childValue: unknown, - strategy: ResolutionStrategy -): unknown { - // Override strategy: child replaces parent completely - if (strategy === 'override') { - return childValue; - } +function warnUnknownKeys(configFile: KeymanConfigFile, configPath: string): void { + const unknown = Object.keys(configFile).filter( + (key) => !KNOWN_KEYS.includes(key as keyof KeymanConfig) + ); - // Merge strategy (default) - if (Array.isArray(parentValue) && Array.isArray(childValue)) { - // Concatenate arrays, remove duplicates for primitives - const combined = [...parentValue, ...childValue]; - if (combined.every((v) => typeof v !== 'object')) { - return [...new Set(combined)]; - } - return combined; + if (unknown.length > 0) { + console.warn( + `⚠️ ${configPath}: ignoring unknown ${unknown.length === 1 ? 'key' : 'keys'} ${unknown.join(', ')}. Known keys: ${KNOWN_KEYS.join(', ')}.` + ); } - - if ( - typeof parentValue === 'object' && - parentValue !== null && - typeof childValue === 'object' && - childValue !== null && - !Array.isArray(parentValue) && - !Array.isArray(childValue) - ) { - // Deep merge objects - const result: Record = { ...parentValue }; - for (const [key, value] of Object.entries(childValue)) { - if (key in result) { - result[key] = mergeValue(result[key], value, 'merge'); - } else { - result[key] = value; - } - } - return result; - } - - // Primitives: child overrides parent - return childValue; } /** - * Merges a child config into a parent config + * Merges a child config into a parent config. + * + * Every property is a string, so a child simply wins. keyman deliberately has + * none of nopy's `resolution` machinery: deep-merge and array-concatenation + * strategies are meaningful there because its config holds arrays and objects, + * and here they would be 45 lines that cannot change an outcome. */ function mergeConfigs(parent: KeymanConfig, childFile: KeymanConfigFile): KeymanConfig { - const resolution = childFile.resolution || {}; - const result: Record = { ...parent }; - - for (const [key, value] of Object.entries(childFile)) { - if (key === 'resolution') continue; // Skip resolution property itself - - const strategy = resolution[key as keyof KeymanConfig] || 'merge'; - if (key in result) { - result[key] = mergeValue(result[key], value, strategy); - } else { - result[key] = value; - } - } - - return result as unknown as KeymanConfig; + return { ...parent, ...childFile }; } /** @@ -183,16 +132,6 @@ function mergeConfigs(parent: KeymanConfig, childFile: KeymanConfigFile): Keyman * Searches for `.keymanrc.json` by traversing upwards from cwd to root. * Multiple config files are merged, with child configs overriding parent configs. * - * Use the `resolution` property to customize merge behavior: - * ```json - * { - * "vaultRoot": "../vault", - * "resolution": { - * "vaultRoot": "override" - * } - * } - * ``` - * * @returns Validated keyman configuration */ export function loadConfig(): KeymanConfig { @@ -211,6 +150,7 @@ export function loadConfig(): KeymanConfig { try { const content = fs.readFileSync(configPath, 'utf-8'); const rawConfig = JSON.parse(content) as KeymanConfigFile; + warnUnknownKeys(rawConfig, configPath); // Resolve path properties relative to the config file's directory const configDir = path.dirname(configPath); const resolvedConfig = resolvePathsRelativeToConfig(rawConfig, configDir); @@ -265,3 +205,17 @@ export function resolveConfigPaths(config: KeymanConfig) { export function getConfigPaths(): string[] { return findConfigFiles(process.cwd()); } + +/** + * What `--print-config` prints. + * + * `configFiles` is the question the flag could not answer before: which files + * were read, in the order they were merged. It existed only as unstructured + * stderr from `loadConfig`, which is exactly the wrong place for it — the JSON is + * the machine-readable half. + */ +export function describeConfig(): ReturnType & { + configFiles: string[]; +} { + return { ...resolveConfigPaths(loadConfig()), configFiles: getConfigPaths() }; +} diff --git a/packages/keyman/tests/config.test.ts b/packages/keyman/tests/config.test.ts index c8cc3c9..4b9a74b 100644 --- a/packages/keyman/tests/config.test.ts +++ b/packages/keyman/tests/config.test.ts @@ -11,6 +11,7 @@ import os from 'node:os'; import path from 'node:path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { + describeConfig, getConfigPaths, type KeymanConfigFile, loadConfig, @@ -208,49 +209,82 @@ describe('keyman config', () => { }); }); - describe('merge strategy', () => { - it('honours an explicit override strategy', () => { - write(rootDir, { vaultRoot: '/parent-vault' }); - const child = path.join(rootDir, 'nested'); - write(child, { vaultRoot: '/child-vault', resolution: { vaultRoot: 'override' } }); - process.chdir(child); + describe('unknown keys', () => { + /** What a config file is likely to get wrong: the casing of a real key. */ + const TYPO = { vaultroot: '/somewhere-else' } as unknown as KeymanConfigFile; - expect(loadConfig().vaultRoot).toBe('/child-vault'); + it('names the file, the key and what it could have been', () => { + write(rootDir, TYPO); + + loadConfig(); + + const warned = messages(warnSpy); + expect(warned).toContain(path.join(rootDir, '.keymanrc.json')); + expect(warned).toContain('vaultroot'); + // Without the list of known keys the warning says a key is wrong without + // saying what right looks like, which for a casing slip is most of the work. + expect(warned).toContain('vaultRoot'); }); - it('never surfaces the resolution key in the loaded config', () => { - write(rootDir, { keysDir: 'my-keys', resolution: { keysDir: 'override' } }); - - expect(loadConfig()).not.toHaveProperty('resolution'); - }); - - it('tolerates and drops array-valued keys the schema does not define', () => { - write(rootDir, { extra: ['a', 'b'] } as unknown as KeymanConfigFile); - const child = path.join(rootDir, 'nested'); - write(child, { extra: ['b', 'c'], keysDir: 'my-keys' } as unknown as KeymanConfigFile); - process.chdir(child); + it('still applies the keys it does understand', () => { + write(rootDir, { ...TYPO, keysDir: 'my-keys' }); const config = loadConfig(); - expect(config).toEqual({ ...DEFAULTS, keysDir: 'my-keys' }); + expect(config.keysDir).toBe('my-keys'); + expect(config.vaultRoot).toBe(DEFAULTS.vaultRoot); }); - it('tolerates and drops object-valued keys the schema does not define', () => { - write(rootDir, { extra: { a: 1 } } as unknown as KeymanConfigFile); + it('blames the file that said it, not the merged result', () => { + write(rootDir, {}); const child = path.join(rootDir, 'nested'); - write(child, { extra: { a: 2, b: 3 } } as unknown as KeymanConfigFile); + write(child, TYPO); process.chdir(child); - expect(loadConfig()).toEqual(DEFAULTS); + loadConfig(); + + expect(messages(warnSpy)).toContain(path.join(child, '.keymanrc.json')); + expect(messages(warnSpy)).not.toContain(path.join(rootDir, '.keymanrc.json')); }); - it('tolerates arrays of objects, which cannot be de-duplicated', () => { - write(rootDir, { extra: [{ a: 1 }] } as unknown as KeymanConfigFile); + it('lists every unknown key in one warning per file', () => { + write(rootDir, { nope: 1, alsoNope: 2 } as unknown as KeymanConfigFile); + + loadConfig(); + + expect(warnSpy).toHaveBeenCalledTimes(1); + expect(messages(warnSpy)).toContain('nope, alsoNope'); + expect(messages(warnSpy)).toContain('unknown keys'); + }); + + it('says key, singular, for one of them', () => { + write(rootDir, TYPO); + + loadConfig(); + + expect(messages(warnSpy)).toContain('unknown key '); + }); + + it('says nothing about a file that sets only known keys', () => { + write(rootDir, { keysDir: 'my-keys', tmpDir: 'my-tmp' }); + + loadConfig(); + + expect(warnSpy).not.toHaveBeenCalled(); + }); + + it.each([ + ['array-valued', { extra: ['a', 'b'] }], + ['object-valued', { extra: { a: 1 } }], + ['an array of objects', { extra: [{ a: 1 }] }], + ])('drops a %s unknown key rather than merging it in', (_label, extra) => { + write(rootDir, extra as unknown as KeymanConfigFile); const child = path.join(rootDir, 'nested'); - write(child, { extra: [{ a: 2 }] } as unknown as KeymanConfigFile); + write(child, { ...extra, keysDir: 'my-keys' } as unknown as KeymanConfigFile); process.chdir(child); - expect(loadConfig()).toEqual(DEFAULTS); + // The schema strips them; nothing in keyman merges an array or an object. + expect(loadConfig()).toEqual({ ...DEFAULTS, keysDir: 'my-keys' }); }); }); @@ -291,4 +325,27 @@ describe('keyman config', () => { expect(paths.keysDir).toBe('/elsewhere/keys'); }); }); + + describe('describeConfig', () => { + it('reports the resolved paths and the files they came from', () => { + write(rootDir, { keysDir: 'my-keys', vaultRoot: 'vault' }); + const child = path.join(rootDir, 'nested'); + write(child, { tmpDir: 'my-tmp' }); + process.chdir(child); + + expect(describeConfig()).toEqual({ + vaultRoot: path.join(rootDir, 'vault'), + keysDir: path.join(rootDir, 'vault', 'my-keys'), + tmpDir: path.join(rootDir, 'vault', 'my-tmp'), + keyPath: path.join(rootDir, 'vault', 'age.key'), + // Parent first, the order they were merged in — which is the only way to + // read a surprising value back to the file responsible for it. + configFiles: [path.join(rootDir, '.keymanrc.json'), path.join(child, '.keymanrc.json')], + }); + }); + + it('reports an empty list when nothing was found', () => { + expect(describeConfig().configFiles).toEqual([]); + }); + }); });