Skip to content

Commit f987cf3

Browse files
zhangsan582TabishB
andauthored
Parse config JSON containers (#1216) (#1244)
Co-authored-by: Tabish Bidiwale <30385142+TabishB@users.noreply.github.com>
1 parent cbf386b commit f987cf3

3 files changed

Lines changed: 90 additions & 1 deletion

File tree

src/core/config-schema.ts

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,13 +146,17 @@ export function deleteNestedValue(obj: Record<string, unknown>, path: string): b
146146
* Coerce a string value to its appropriate type.
147147
* - "true" / "false" -> boolean
148148
* - Numeric strings -> number
149+
* - JSON arrays/objects -> parsed containers
149150
* - Everything else -> string
150151
*
151152
* @param value - The string value to coerce
152153
* @param forceString - If true, always return the value as a string
153154
* @returns The coerced value
154155
*/
155-
export function coerceValue(value: string, forceString: boolean = false): string | number | boolean {
156+
export function coerceValue(
157+
value: string,
158+
forceString: boolean = false
159+
): string | number | boolean | unknown[] | Record<string, unknown> {
156160
if (forceString) {
157161
return value;
158162
}
@@ -171,9 +175,39 @@ export function coerceValue(value: string, forceString: boolean = false): string
171175
return num;
172176
}
173177

178+
const jsonContainer = parseJsonContainer(value);
179+
if (jsonContainer !== undefined) {
180+
return jsonContainer;
181+
}
182+
174183
return value;
175184
}
176185

186+
function parseJsonContainer(value: string): unknown[] | Record<string, unknown> | undefined {
187+
const trimmed = value.trim();
188+
const looksLikeContainer =
189+
(trimmed.startsWith('[') && trimmed.endsWith(']')) ||
190+
(trimmed.startsWith('{') && trimmed.endsWith('}'));
191+
192+
if (!looksLikeContainer) {
193+
return undefined;
194+
}
195+
196+
try {
197+
const parsed: unknown = JSON.parse(trimmed);
198+
if (Array.isArray(parsed)) {
199+
return parsed;
200+
}
201+
if (parsed !== null && typeof parsed === 'object') {
202+
return parsed as Record<string, unknown>;
203+
}
204+
} catch {
205+
return undefined;
206+
}
207+
208+
return undefined;
209+
}
210+
177211
/**
178212
* Format a value for YAML-like display.
179213
*

test/commands/config.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,22 @@
11
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
2+
import { Command } from 'commander';
23
import * as fs from 'node:fs';
34
import * as path from 'node:path';
45
import * as os from 'node:os';
56

7+
async function runConfigCommand(args: string[]): Promise<void> {
8+
const { registerConfigCommand } = await import('../../src/commands/config.js');
9+
const program = new Command();
10+
registerConfigCommand(program);
11+
await program.parseAsync(['node', 'openspec', 'config', ...args]);
12+
}
13+
614
describe('config command integration', () => {
715
// These tests use real file system operations with XDG_CONFIG_HOME override
816
let tempDir: string;
917
let originalEnv: NodeJS.ProcessEnv;
1018
let consoleErrorSpy: ReturnType<typeof vi.spyOn>;
19+
let consoleLogSpy: ReturnType<typeof vi.spyOn>;
1120

1221
beforeEach(() => {
1322
// Create unique temp directory for each test
@@ -20,6 +29,7 @@ describe('config command integration', () => {
2029

2130
// Spy on console.error
2231
consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
32+
consoleLogSpy = vi.spyOn(console, 'log').mockImplementation(() => {});
2333
});
2434

2535
afterEach(() => {
@@ -31,6 +41,7 @@ describe('config command integration', () => {
3141

3242
// Restore spies
3343
consoleErrorSpy.mockRestore();
44+
consoleLogSpy.mockRestore();
3445

3546
// Reset module cache to pick up new XDG_CONFIG_HOME
3647
vi.resetModules();
@@ -89,6 +100,22 @@ describe('config command integration', () => {
89100
expect(config.featureFlags).toEqual({});
90101
expect(consoleErrorSpy).toHaveBeenCalledWith(expect.stringContaining('Invalid JSON'));
91102
});
103+
104+
it('should set workflows from JSON array syntax', async () => {
105+
await runConfigCommand([
106+
'set',
107+
'workflows',
108+
'["new","ff","apply","archive"]',
109+
]);
110+
111+
const { getGlobalConfig } = await import('../../src/core/global-config.js');
112+
const config = getGlobalConfig();
113+
114+
expect(config.workflows).toEqual(['new', 'ff', 'apply', 'archive']);
115+
expect(consoleLogSpy).toHaveBeenCalledWith(
116+
'Set workflows = new,ff,apply,archive'
117+
);
118+
});
92119
});
93120

94121
describe('config command shell completion registry', () => {

test/core/config-schema.test.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,23 @@ describe('config-schema', () => {
151151
expect(coerceValue('hello')).toBe('hello');
152152
});
153153

154+
it('should parse JSON arrays', () => {
155+
expect(coerceValue('["new","ff","apply","archive"]')).toEqual([
156+
'new',
157+
'ff',
158+
'apply',
159+
'archive',
160+
]);
161+
});
162+
163+
it('should parse JSON objects', () => {
164+
expect(coerceValue('{"nested":"value"}')).toEqual({ nested: 'value' });
165+
});
166+
167+
it('should keep malformed JSON containers as strings', () => {
168+
expect(coerceValue('["new",')).toBe('["new",');
169+
});
170+
154171
it('should keep strings that start with numbers but are not numbers', () => {
155172
expect(coerceValue('123abc')).toBe('123abc');
156173
});
@@ -167,6 +184,7 @@ describe('config-schema', () => {
167184
expect(coerceValue('true', true)).toBe('true');
168185
expect(coerceValue('42', true)).toBe('42');
169186
expect(coerceValue('hello', true)).toBe('hello');
187+
expect(coerceValue('["new"]', true)).toBe('["new"]');
170188
});
171189

172190
it('should not coerce Infinity to number (not finite)', () => {
@@ -318,6 +336,16 @@ describe('config-schema', () => {
318336
expect(result.success).toBe(true);
319337
expect((config.featureFlags as Record<string, unknown>).experimental).toBe(false);
320338
});
339+
340+
it('should accept setting workflows from JSON array syntax', () => {
341+
const config: Record<string, unknown> = { featureFlags: {}, profile: 'custom' };
342+
const value = coerceValue('["new","ff","apply","archive"]');
343+
setNestedValue(config, 'workflows', value);
344+
345+
const result = validateConfig(config);
346+
expect(result.success).toBe(true);
347+
expect(config.workflows).toEqual(['new', 'ff', 'apply', 'archive']);
348+
});
321349
});
322350

323351
describe('GlobalConfigSchema', () => {

0 commit comments

Comments
 (0)