Skip to content

Commit 6c8c778

Browse files
authored
fix(config): handle null rules field in project config (Fission-AI#529)
* feat(config): add project-level configuration via openspec/config.yaml Adds openspec/config.yaml support for project-level customization without forking schemas. Teams can now: - Set a default schema (used when --schema flag not provided) - Inject project context into all artifact instructions - Add per-artifact rules (e.g., proposal rules, specs rules) Key changes: - New `src/core/project-config.ts` with Zod schema and resilient parsing - New `src/core/config-prompts.ts` for interactive config creation - Updated schema resolution order: CLI → change metadata → config → default - Updated instruction generation to inject <context> and <rules> XML sections - Integrated config creation prompts into `artifact-experimental-setup` Schema resolution precedence: 1. --schema CLI flag (explicit override) 2. .openspec.yaml in change directory (change-specific) 3. openspec/config.yaml schema field (project default) 4. "spec-driven" (hardcoded fallback) * test(config): add e2e tests and performance benchmarks for project config - Add project config integration tests in artifact-workflow.test.ts: - Test new change uses schema from config - Test CLI schema overrides config schema - Test context and rules injection in instructions - Test backwards compatibility without config file - Test immediate reflection of config changes - Add performance benchmark tests in project-config.test.ts: - Typical config (1KB): <20ms target - Large config (50KB): <50ms target - Repeated reads consistency check - Missing config fast-path test - Add project configuration documentation in experimental-workflow.md: - Config fields reference (schema, context, rules) - Schema precedence explanation - Artifact IDs by schema - Troubleshooting guide - Mark all tasks complete in tasks.md * refactor(config): remove benchmark tests, document decision in source Remove performance benchmark tests from project-config.test.ts and document the results directly in the source code instead. Benchmarks showed config reads are fast enough (~0.5ms typical) that caching is unnecessary. * fix(config): resolve three issues in project config feature - Remove nested <template> tags: generateInstructions() no longer wraps template content since printInstructionsText() already handles XML structure - Fix schema message accuracy: newChangeCommand() now uses the resolved schema returned from createChange() instead of hardcoded fallback - Add TTY check: artifact-experimental-setup skips interactive prompts in non-TTY environments (CI, automation) to prevent hangs * fix(config): handle null rules field in project config Add null check when parsing rules field since YAML `rules:` with no value parses to null, and `typeof null === 'object'` in JavaScript. Without this check, Object.entries(null) would throw an error.
1 parent 43b01ad commit 6c8c778

2 files changed

Lines changed: 26 additions & 2 deletions

File tree

src/core/project-config.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,8 @@ export function readProjectConfig(projectRoot: string): ProjectConfig | null {
117117
if (raw.rules !== undefined) {
118118
const rulesField = z.record(z.string(), z.array(z.string()));
119119

120-
// First check if it's an object structure
121-
if (typeof raw.rules === 'object' && !Array.isArray(raw.rules)) {
120+
// First check if it's an object structure (guard against null since typeof null === 'object')
121+
if (typeof raw.rules === 'object' && raw.rules !== null && !Array.isArray(raw.rules)) {
122122
const parsedRules: Record<string, string[]> = {};
123123
let hasValidRules = false;
124124

test/core/project-config.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,30 @@ rules: ["not", "an", "object"]
142142
);
143143
});
144144

145+
it('should handle rules: null without aborting config parsing', () => {
146+
// YAML `rules:` with no value parses to null
147+
const configDir = path.join(tempDir, 'openspec');
148+
fs.mkdirSync(configDir, { recursive: true });
149+
fs.writeFileSync(
150+
path.join(configDir, 'config.yaml'),
151+
`schema: spec-driven
152+
context: Valid context
153+
rules:
154+
`
155+
);
156+
157+
const config = readProjectConfig(tempDir);
158+
159+
// Should still parse schema and context despite null rules
160+
expect(config).toEqual({
161+
schema: 'spec-driven',
162+
context: 'Valid context',
163+
});
164+
expect(consoleWarnSpy).toHaveBeenCalledWith(
165+
expect.stringContaining("Invalid 'rules' field")
166+
);
167+
});
168+
145169
it('should filter out invalid rules for specific artifact', () => {
146170
const configDir = path.join(tempDir, 'openspec');
147171
fs.mkdirSync(configDir, { recursive: true });

0 commit comments

Comments
 (0)