From 3359c860eb4373ab108ecdbdf5828189e6129b4c Mon Sep 17 00:00:00 2001 From: jinzelin Date: Mon, 25 May 2026 14:47:13 +0800 Subject: [PATCH 1/2] fix: config set support object value --- src/commands/config.ts | 92 ++++++++++++++++++++++++++++-------------- 1 file changed, 62 insertions(+), 30 deletions(-) diff --git a/src/commands/config.ts b/src/commands/config.ts index 871d3a085..1c1a49e7c 100644 --- a/src/commands/config.ts +++ b/src/commands/config.ts @@ -351,42 +351,74 @@ export function registerConfigCommand(program: Command): void { .command('set ') .description('Set a value (auto-coerce types)') .option('--string', 'Force value to be stored as string') + .option('--json', 'Parse value as JSON (use for arrays/objects)') .option('--allow-unknown', 'Allow setting unknown keys') - .action((key: string, value: string, options: { string?: boolean; allowUnknown?: boolean }) => { - const allowUnknown = Boolean(options.allowUnknown); - const keyValidation = validateConfigKeyPath(key); - if (!keyValidation.valid && !allowUnknown) { - const reason = keyValidation.reason ? ` ${keyValidation.reason}.` : ''; - console.error(`Error: Invalid configuration key "${key}".${reason}`); - console.error('Use "openspec config list" to see available keys.'); - console.error('Pass --allow-unknown to bypass this check.'); - process.exitCode = 1; - return; - } + .action( + ( + key: string, + value: string, + options: { string?: boolean; json?: boolean; allowUnknown?: boolean } + ) => { + const allowUnknown = Boolean(options.allowUnknown); + const keyValidation = validateConfigKeyPath(key); + if (!keyValidation.valid && !allowUnknown) { + const reason = keyValidation.reason ? ` ${keyValidation.reason}.` : ''; + console.error(`Error: Invalid configuration key "${key}".${reason}`); + console.error('Use "openspec config list" to see available keys.'); + console.error('Pass --allow-unknown to bypass this check.'); + process.exitCode = 1; + return; + } - const config = getGlobalConfig() as Record; - const coercedValue = coerceValue(value, options.string || false); + if (options.string && options.json) { + console.error('Error: --string and --json cannot be combined.'); + process.exitCode = 1; + return; + } - // Create a copy to validate before saving - const newConfig = JSON.parse(JSON.stringify(config)); - setNestedValue(newConfig, key, coercedValue); + let parsedValue: unknown; + if (options.json) { + try { + parsedValue = JSON.parse(value); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + console.error(`Error: --json was provided but value is not valid JSON: ${message}`); + process.exitCode = 1; + return; + } + } else { + parsedValue = coerceValue(value, options.string || false); + } - // Validate the new config - const validation = validateConfig(newConfig); - if (!validation.success) { - console.error(`Error: Invalid configuration - ${validation.error}`); - process.exitCode = 1; - return; - } + const config = getGlobalConfig() as Record; - // Apply changes and save - setNestedValue(config, key, coercedValue); - saveGlobalConfig(config as GlobalConfig); + // Create a copy to validate before saving + const newConfig = JSON.parse(JSON.stringify(config)); + setNestedValue(newConfig, key, parsedValue); - const displayValue = - typeof coercedValue === 'string' ? `"${coercedValue}"` : String(coercedValue); - console.log(`Set ${key} = ${displayValue}`); - }); + // Validate the new config + const validation = validateConfig(newConfig); + if (!validation.success) { + console.error(`Error: Invalid configuration - ${validation.error}`); + process.exitCode = 1; + return; + } + + // Apply changes and save + setNestedValue(config, key, parsedValue); + saveGlobalConfig(config as GlobalConfig); + + let displayValue: string; + if (typeof parsedValue === 'string') { + displayValue = `"${parsedValue}"`; + } else if (typeof parsedValue === 'object' && parsedValue !== null) { + displayValue = JSON.stringify(parsedValue); + } else { + displayValue = String(parsedValue); + } + console.log(`Set ${key} = ${displayValue}`); + } + ); // config unset configCmd From 57d5e1fa96d5074db4697d7b0d0d2760a2177efd Mon Sep 17 00:00:00 2001 From: jinzelin Date: Thu, 4 Jun 2026 20:58:49 +0800 Subject: [PATCH 2/2] fix: config set support object value: add test --- src/core/completions/command-registry.ts | 4 + test/commands/config-set-json.test.ts | 307 +++++++++++++++++++++++ 2 files changed, 311 insertions(+) create mode 100644 test/commands/config-set-json.test.ts diff --git a/src/core/completions/command-registry.ts b/src/core/completions/command-registry.ts index 88ec88e05..71f0cb3df 100644 --- a/src/core/completions/command-registry.ts +++ b/src/core/completions/command-registry.ts @@ -833,6 +833,10 @@ export const COMMAND_REGISTRY: CommandDefinition[] = [ name: 'string', description: 'Force value to be stored as string', }, + { + name: 'json', + description: 'Parse value as JSON (use for arrays/objects)', + }, { name: 'allow-unknown', description: 'Allow setting unknown keys', diff --git a/test/commands/config-set-json.test.ts b/test/commands/config-set-json.test.ts new file mode 100644 index 000000000..2e39d6f1d --- /dev/null +++ b/test/commands/config-set-json.test.ts @@ -0,0 +1,307 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { Command } from 'commander'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import * as os from 'node:os'; + +/** + * Focused tests for `openspec config set` and its --json flag. + * + * Covers: + * - JSON arrays / objects via --json + * - Invalid JSON (with --json) reports the parse error and does not write + * - --string and --json conflict + * - Schema validation failure (e.g. workflows must be string[]) + * - Output formatting for strings / numbers / booleans / arrays / objects + * - No mutation of the config file on any error path + * - Shell completion registry exposes --json on `config set` + */ + +async function runConfigCommand(args: string[]): Promise { + const { registerConfigCommand } = await import('../../src/commands/config.js'); + const program = new Command(); + // commander would otherwise call process.exit() on parse errors; make it throw + // so the test runner can surface a real failure instead of killing the process. + program.exitOverride(); + registerConfigCommand(program); + await program.parseAsync(['node', 'openspec', 'config', ...args]); +} + +describe('config set --json', () => { + let tempDir: string; + let originalEnv: NodeJS.ProcessEnv; + let originalExitCode: number | undefined; + let consoleLogSpy: ReturnType; + let consoleErrorSpy: ReturnType; + + function getConfigFilePath(): string { + return path.join(tempDir, 'openspec', 'config.json'); + } + + function snapshotConfigFile(): string | null { + const p = getConfigFilePath(); + return fs.existsSync(p) ? fs.readFileSync(p, 'utf-8') : null; + } + + function readConfigJson(): Record { + return JSON.parse(fs.readFileSync(getConfigFilePath(), 'utf-8')); + } + + function getLoggedLines(spy: ReturnType): string[] { + return spy.mock.calls.map((args) => args.map(String).join(' ')); + } + + beforeEach(() => { + vi.resetModules(); + + tempDir = path.join( + os.tmpdir(), + `openspec-config-set-json-test-${Date.now()}-${Math.random().toString(36).slice(2)}` + ); + fs.mkdirSync(tempDir, { recursive: true }); + + originalEnv = { ...process.env }; + process.env.XDG_CONFIG_HOME = tempDir; + + originalExitCode = process.exitCode; + process.exitCode = undefined; + + consoleLogSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + }); + + afterEach(() => { + process.env = originalEnv; + process.exitCode = originalExitCode; + fs.rmSync(tempDir, { recursive: true, force: true }); + consoleLogSpy.mockRestore(); + consoleErrorSpy.mockRestore(); + vi.clearAllMocks(); + }); + + describe('JSON arrays / objects', () => { + it('parses a JSON array of strings into workflows', async () => { + await runConfigCommand([ + 'set', + 'workflows', + '["propose","explore","apply","sync","archive"]', + '--json', + ]); + + expect(process.exitCode).toBeFalsy(); + expect(readConfigJson().workflows).toEqual([ + 'propose', + 'explore', + 'apply', + 'sync', + 'archive', + ]); + expect(getLoggedLines(consoleLogSpy)).toContain( + 'Set workflows = ["propose","explore","apply","sync","archive"]' + ); + }); + + it('parses a JSON object into featureFlags', async () => { + await runConfigCommand([ + 'set', + 'featureFlags', + '{"alpha":true,"beta":false}', + '--json', + ]); + + expect(process.exitCode).toBeFalsy(); + expect(readConfigJson().featureFlags).toEqual({ alpha: true, beta: false }); + expect(getLoggedLines(consoleLogSpy)).toContain( + 'Set featureFlags = {"alpha":true,"beta":false}' + ); + }); + + it('accepts an empty JSON array', async () => { + await runConfigCommand(['set', 'workflows', '[]', '--json']); + + expect(process.exitCode).toBeFalsy(); + expect(readConfigJson().workflows).toEqual([]); + expect(getLoggedLines(consoleLogSpy)).toContain('Set workflows = []'); + }); + }); + + describe('invalid JSON with --json', () => { + it('reports a parse error and does not write the config file', async () => { + const before = snapshotConfigFile(); + + await runConfigCommand(['set', 'workflows', '[propose, apply]', '--json']); + + expect(process.exitCode).toBe(1); + expect(snapshotConfigFile()).toBe(before); + const errors = getLoggedLines(consoleErrorSpy); + expect( + errors.some((line) => + line.startsWith('Error: --json was provided but value is not valid JSON:') + ) + ).toBe(true); + expect(consoleLogSpy).not.toHaveBeenCalled(); + }); + + it('reports a parse error for truncated JSON object', async () => { + const before = snapshotConfigFile(); + + await runConfigCommand(['set', 'featureFlags', '{"a":', '--json']); + + expect(process.exitCode).toBe(1); + expect(snapshotConfigFile()).toBe(before); + }); + }); + + describe('--string / --json conflict', () => { + it('errors out when both flags are provided', async () => { + const before = snapshotConfigFile(); + + await runConfigCommand([ + 'set', + 'workflows', + '["propose"]', + '--string', + '--json', + ]); + + expect(process.exitCode).toBe(1); + expect(snapshotConfigFile()).toBe(before); + expect(getLoggedLines(consoleErrorSpy)).toContain( + 'Error: --string and --json cannot be combined.' + ); + expect(consoleLogSpy).not.toHaveBeenCalled(); + }); + }); + + describe('schema validation failure', () => { + it('rejects a JSON value whose shape does not match the schema and does not write', async () => { + // workflows is array; passing a JSON object should fail schema validation + const before = snapshotConfigFile(); + + await runConfigCommand([ + 'set', + 'workflows', + '{"not":"an array"}', + '--json', + ]); + + expect(process.exitCode).toBe(1); + expect(snapshotConfigFile()).toBe(before); + const errors = getLoggedLines(consoleErrorSpy); + expect( + errors.some((line) => line.startsWith('Error: Invalid configuration')) + ).toBe(true); + expect(errors.some((line) => line.toLowerCase().includes('workflows'))).toBe(true); + expect(consoleLogSpy).not.toHaveBeenCalled(); + }); + + it('rejects an invalid enum value (delivery) and does not write', async () => { + const before = snapshotConfigFile(); + + await runConfigCommand(['set', 'delivery', 'invalid-mode']); + + expect(process.exitCode).toBe(1); + expect(snapshotConfigFile()).toBe(before); + const errors = getLoggedLines(consoleErrorSpy); + expect( + errors.some((line) => line.startsWith('Error: Invalid configuration')) + ).toBe(true); + }); + + it('rejects an unknown top-level key without --allow-unknown', async () => { + const before = snapshotConfigFile(); + + await runConfigCommand(['set', 'mysteryKey', '"value"', '--json']); + + expect(process.exitCode).toBe(1); + expect(snapshotConfigFile()).toBe(before); + const errors = getLoggedLines(consoleErrorSpy); + expect( + errors.some((line) => line.startsWith('Error: Invalid configuration key "mysteryKey"')) + ).toBe(true); + }); + + it('does not mutate previously stored value when a later set fails validation', async () => { + // First write a known-good workflows value. + await runConfigCommand([ + 'set', + 'workflows', + '["propose","apply"]', + '--json', + ]); + expect(readConfigJson().workflows).toEqual(['propose', 'apply']); + const goodSnapshot = snapshotConfigFile(); + + // Now try a value that the schema must reject. + await runConfigCommand([ + 'set', + 'workflows', + '[1,2,3]', + '--json', + ]); + + expect(process.exitCode).toBe(1); + // File on disk must be untouched. + expect(snapshotConfigFile()).toBe(goodSnapshot); + // In-memory read should still return the original good value. + expect(readConfigJson().workflows).toEqual(['propose', 'apply']); + }); + }); + + describe('output formatting', () => { + it('quotes plain string values', async () => { + await runConfigCommand(['set', 'profile', 'custom']); + expect(getLoggedLines(consoleLogSpy)).toContain('Set profile = "custom"'); + }); + + it('renders boolean values without quotes', async () => { + await runConfigCommand(['set', 'featureFlags.alpha', 'true']); + expect(getLoggedLines(consoleLogSpy)).toContain('Set featureFlags.alpha = true'); + expect(readConfigJson().featureFlags).toEqual({ alpha: true }); + }); + + it('renders arrays via JSON.stringify', async () => { + await runConfigCommand(['set', 'workflows', '["propose"]', '--json']); + expect(getLoggedLines(consoleLogSpy)).toContain('Set workflows = ["propose"]'); + }); + + it('renders objects via JSON.stringify', async () => { + await runConfigCommand(['set', 'featureFlags', '{"x":true}', '--json']); + expect(getLoggedLines(consoleLogSpy)).toContain('Set featureFlags = {"x":true}'); + }); + + it('--string keeps a number-looking value as a quoted string', async () => { + // featureFlags.alpha must be boolean per schema, so use an unknown key with --allow-unknown + // to exercise --string formatting independently of schema typing. + await runConfigCommand([ + 'set', + 'someText', + '42', + '--string', + '--allow-unknown', + ]); + expect(getLoggedLines(consoleLogSpy)).toContain('Set someText = "42"'); + expect(readConfigJson().someText).toBe('42'); + }); + }); +}); + +describe('config set shell completion registry', () => { + it('exposes --json alongside --string and --allow-unknown', async () => { + const { COMMAND_REGISTRY } = await import( + '../../src/core/completions/command-registry.js' + ); + + const configCmd = COMMAND_REGISTRY.find((cmd) => cmd.name === 'config'); + const setCmd = configCmd?.subcommands?.find((s) => s.name === 'set'); + expect(setCmd).toBeDefined(); + + const flagNames = setCmd?.flags?.map((f) => f.name) ?? []; + expect(flagNames).toContain('string'); + expect(flagNames).toContain('json'); + expect(flagNames).toContain('allow-unknown'); + + const jsonFlag = setCmd?.flags?.find((f) => f.name === 'json'); + expect(jsonFlag?.description).toMatch(/json/i); + }); +});