diff --git a/docs/cli.md b/docs/cli.md index d17c6d662f..50e234e3fd 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -834,9 +834,9 @@ openspec instructions design --change add-dark-mode --json **Output includes:** - Template content for the artifact -- Project context from config +- Project context from config, plus any `schemas..context` override for the resolved schema (see [Customization](customization.md#project-configuration)) - Content from dependency artifacts -- Per-artifact rules from config +- Per-artifact rules from config, plus any matching `schemas..rules` override for the resolved schema - Current project context and matching operation guidance for `apply`/`archive` Operation inputs are read from the resolved repo or selected store on every diff --git a/docs/customization.md b/docs/customization.md index b1143b9276..065207c65f 100644 --- a/docs/customization.md +++ b/docs/customization.md @@ -95,6 +95,42 @@ Tech stack: TypeScript, React, Node.js, PostgreSQL - **Context** appears in ALL artifacts - **Rules** ONLY appear for the matching artifact +**Scoping context/rules to one schema:** + +The top-level `context` and `rules` above apply to every schema in the +project - useful, but a problem if you register more than one schema (say, +the built-in `spec-driven` plus a schema forked for a different team) and +want a rule or note to apply to only one of them. Add an optional `schemas` +block, keyed by schema name, to layer schema-specific context and rules on +top of the project-wide ones: + +```yaml +# openspec/config.yaml +schema: spec-driven + +context: | + Tech stack: TypeScript, React, Node.js, PostgreSQL + +rules: + proposal: + - Include rollback plan + +schemas: + my-workflow: + context: | + This schema is for the payments team; flag PCI-scope changes explicitly. + rules: + proposal: + - Tag the proposal with the affected payment provider +``` + +Generating instructions for `my-workflow`'s `proposal` artifact gets the +project-wide context and rule above **plus** the `my-workflow`-only context +and rule; generating instructions for `spec-driven`'s `proposal` artifact +(or any other schema) only gets the project-wide ones. A schema with no +entry under `schemas` is unaffected - existing `config.yaml` files that only +use the flat `context`/`rules` fields keep behaving exactly as before. + **Operation guidance:** `operations.apply.guidance` and `operations.archive.guidance` are optional arrays diff --git a/src/core/artifact-graph/instruction-loader.ts b/src/core/artifact-graph/instruction-loader.ts index 3f12670016..90f61c34d6 100644 --- a/src/core/artifact-graph/instruction-loader.ts +++ b/src/core/artifact-graph/instruction-loader.ts @@ -13,7 +13,13 @@ import { type ActionContext, type PlanningHomeSummary, } from '../change-status-policy.js'; -import { readProjectConfig, validateConfigRules, type ProjectConfig } from '../project-config.js'; +import { + readProjectConfig, + validateConfigRules, + resolveEffectiveContext, + resolveEffectiveRules, + type ProjectConfig, +} from '../project-config.js'; import type { ReferenceIndexEntry } from '../references.js'; import type { PlanningHome } from '../planning-home.js'; import type { ChangeMetadata } from '../change-metadata/index.js'; @@ -360,13 +366,42 @@ export function generateInstructions( } // Validate rules artifact IDs if config has rules (only once per session). - // The rules map is global while each change can use a different schema, so a - // key is only "unknown" when it matches no artifact in ANY available schema. - if (projectConfig?.rules) { - const validArtifactIds = new Set( - listSchemasWithInfo(effectiveProjectRoot ?? undefined).flatMap((s) => s.artifacts) - ); - const warnings = validateConfigRules(projectConfig.rules, validArtifactIds); + // The global rules map applies while each change can use a different + // schema, so a key is only "unknown" when it matches no artifact in ANY + // available schema. Schema-scoped `schemas..rules` are checked more + // precisely, against just that schema's own artifacts, since they only + // ever apply there. + if (projectConfig?.rules || projectConfig?.schemas) { + const schemasWithInfo = listSchemasWithInfo(effectiveProjectRoot ?? undefined); + const warnings: string[] = []; + + if (projectConfig.rules) { + const validArtifactIds = new Set(schemasWithInfo.flatMap((s) => s.artifacts)); + warnings.push(...validateConfigRules(projectConfig.rules, validArtifactIds)); + } + + if (projectConfig.schemas) { + const artifactIdsBySchema = new Map( + schemasWithInfo.map((s) => [s.name, new Set(s.artifacts)]) + ); + for (const [scopedSchemaName, scoped] of Object.entries(projectConfig.schemas)) { + const scopedValidIds = artifactIdsBySchema.get(scopedSchemaName); + if (!scopedValidIds) { + // A typo'd or removed schema name here never matches any active + // schema, so its context/rules silently never apply - warn instead + // of leaving that dead config unexplained. + const knownSchemas = Array.from(artifactIdsBySchema.keys()).sort().join(', '); + warnings.push( + `Unknown schema '${scopedSchemaName}' in config 'schemas'. ` + + `It matches no available schema, so its context/rules overrides are ignored. Known schemas: ${knownSchemas}` + ); + continue; + } + if (scoped.rules) { + warnings.push(...validateConfigRules(scoped.rules, scopedValidIds, scopedSchemaName)); + } + } + } // Show each unique warning only once per session for (const warning of warnings) { @@ -377,13 +412,12 @@ export function generateInstructions( } } - // Extract context and rules as separate fields (not prepended to template) - const configContext = projectConfig?.context?.trim() || undefined; - const rulesForArtifact = - projectConfig?.rules && Object.hasOwn(projectConfig.rules, artifactId) - ? projectConfig.rules[artifactId] - : undefined; - const configRules = rulesForArtifact && rulesForArtifact.length > 0 ? rulesForArtifact : undefined; + // Extract context and rules as separate fields (not prepended to template). + // Both layer the schema-scoped `schemas.` override (if any) on + // top of the project-level value, so single-schema projects that only use + // the flat `context`/`rules` fields keep behaving exactly as before. + const configContext = resolveEffectiveContext(projectConfig, context.schemaName); + const configRules = resolveEffectiveRules(projectConfig, context.schemaName, artifactId); return { changeName: context.changeName, diff --git a/src/core/project-config.ts b/src/core/project-config.ts index 922e31505b..c8af6aed46 100644 --- a/src/core/project-config.ts +++ b/src/core/project-config.ts @@ -52,6 +52,22 @@ export const ProjectConfigSchema = z.object({ .optional() .describe('Per-artifact rules, keyed by artifact ID'), + // Optional: schema-scoped context/rules overrides, keyed by schema name. + // A project can register multiple schemas (built-in plus forked/local + // ones); the `context`/`rules` fields above apply to all of them, and this + // is the only way to add context or rules that apply to just one schema. + // Layered on top of (additive to) the fields above, never a replacement. + schemas: z + .record( + z.string(), // schema name + z.object({ + context: z.string().optional(), + rules: z.record(z.string(), z.array(z.string())).optional(), + }) + ) + .optional() + .describe('Schema-scoped context/rules overrides, keyed by schema name'), + // Optional: per-operation advisory guidance, kept separate from artifact rules. operations: z .object({ @@ -92,6 +108,12 @@ export interface DeclarationEntry { remote?: string; } +/** Schema-scoped `context`/`rules` override; see `schemas` on ProjectConfigSchema. */ +export interface ScopedProjectConfig { + context?: string; + rules?: Record; +} + export type ProjectConfig = z.infer & { references?: DeclarationEntry[]; }; @@ -180,6 +202,105 @@ function parseOperations(raw: unknown): OperationsConfig | undefined { return Object.keys(operations).length > 0 ? operations : undefined; } +/** + * Parser for `schemas:` overrides: an object keyed by schema name, each + * value optionally carrying its own `context` string and `rules` map. Mirrors + * the resilient, warn-and-drop parsing used for the top-level `context` and + * `rules` fields (including the null-prototype guard on rule keys), just + * applied once per schema name. + */ +function parseScopedSchemas(raw: unknown): Record | undefined { + if (raw === undefined) { + return undefined; + } + if (typeof raw !== 'object' || raw === null || Array.isArray(raw)) { + console.warn(`Invalid 'schemas' field in config (must be object)`); + return undefined; + } + + const parsedSchemas: Record = Object.create(null); + let hasValidSchemas = false; + + for (const [schemaName, value] of Object.entries(raw)) { + if (!value || typeof value !== 'object' || Array.isArray(value)) { + console.warn( + `Invalid 'schemas.${schemaName}' field in config (must be object), ignoring this schema's overrides` + ); + continue; + } + + const scopedRaw = value as Record; + const unknownFields = Object.keys(scopedRaw).filter( + (field) => field !== 'context' && field !== 'rules' + ); + if (unknownFields.length > 0) { + console.warn( + `Unknown field(s) in 'schemas.${schemaName}': ${unknownFields.join(', ')}. Supported fields: context, rules` + ); + } + + const scoped: ScopedProjectConfig = {}; + + if (scopedRaw.context !== undefined) { + const contextResult = z.string().safeParse(scopedRaw.context); + if (contextResult.success) { + const contextSize = Buffer.byteLength(contextResult.data, 'utf-8'); + if (contextSize > MAX_CONTEXT_SIZE) { + console.warn( + `Context too large in 'schemas.${schemaName}.context' (${(contextSize / 1024).toFixed(1)}KB, limit: ${MAX_CONTEXT_SIZE / 1024}KB)` + ); + console.warn(`Ignoring 'schemas.${schemaName}.context' field`); + } else { + scoped.context = contextResult.data; + } + } else { + console.warn(`Invalid 'schemas.${schemaName}.context' field in config (must be string)`); + } + } + + if (scopedRaw.rules !== undefined) { + if (typeof scopedRaw.rules === 'object' && scopedRaw.rules !== null && !Array.isArray(scopedRaw.rules)) { + const parsedRules: Record = Object.create(null); + let hasValidRules = false; + + for (const [artifactId, rules] of Object.entries(scopedRaw.rules)) { + const rulesArrayResult = z.array(z.string()).safeParse(rules); + + if (rulesArrayResult.success) { + const validRules = rulesArrayResult.data.filter((r) => r.length > 0); + if (validRules.length > 0) { + parsedRules[artifactId] = validRules; + hasValidRules = true; + } + if (validRules.length < rulesArrayResult.data.length) { + console.warn( + `Some rules for '${artifactId}' in 'schemas.${schemaName}.rules' are empty strings, ignoring them` + ); + } + } else { + console.warn( + `Rules for '${artifactId}' in 'schemas.${schemaName}.rules' must be an array of strings, ignoring this artifact's rules` + ); + } + } + + if (hasValidRules) { + scoped.rules = parsedRules; + } + } else { + console.warn(`Invalid 'schemas.${schemaName}.rules' field in config (must be object)`); + } + } + + if (scoped.context !== undefined || scoped.rules !== undefined) { + parsedSchemas[schemaName] = scoped; + hasValidSchemas = true; + } + } + + return hasValidSchemas ? parsedSchemas : undefined; +} + /** * Parser for `references:` declarations: string entries or * {id, remote} maps, normalized to DeclarationEntry[]. Dedup keys on @@ -353,6 +474,11 @@ export function readProjectConfig(projectRoot: string): ProjectConfig | null { } } + const schemas = parseScopedSchemas(raw.schemas); + if (schemas) { + config.schemas = schemas; + } + const operations = parseOperations(raw.operations); if (operations) { config.operations = operations; @@ -409,27 +535,38 @@ function configPathForWarnings(projectRoot: string): string { } /** - * Validate artifact IDs in rules against the artifacts of every available - * schema. The `rules:` map is global, but each change can use a different - * schema, so a key is only unknown when it matches no artifact in ANY schema. - * Returns warnings for keys that are unknown everywhere. + * Validate artifact IDs in rules against a set of valid artifact IDs. + * Returns warnings for keys that don't match. + * + * Called two ways: + * - Global `rules:` map: pass the union of artifact IDs across every + * available schema (a key is only unknown when it matches no artifact in + * ANY schema, since a change can use a different schema than the one + * active when the warning is produced). Omit `schemaName`. + * - Schema-scoped `schemas..rules:` map: pass just that schema's own + * artifact IDs and its name, since scoped rules are only ever applied + * there - an unknown key can be reported precisely. * * @param rules - The rules object from config - * @param validArtifactIds - Set of valid artifact IDs across all schemas + * @param validArtifactIds - Set of valid artifact IDs to check against + * @param schemaName - When set, the warning is worded for this one schema + * instead of "any available schema" * @returns Array of warning messages for unknown artifact IDs */ export function validateConfigRules( rules: Record, - validArtifactIds: Set + validArtifactIds: Set, + schemaName?: string ): string[] { const warnings: string[] = []; + const scopeLabel = schemaName ? `schema '${schemaName}'` : 'any available schema'; for (const artifactId of Object.keys(rules)) { if (!validArtifactIds.has(artifactId)) { const validIds = Array.from(validArtifactIds).sort().join(', '); warnings.push( `Unknown artifact ID in rules: "${artifactId}". ` + - `It matches no artifact in any available schema. Known artifact IDs: ${validIds}` + `It matches no artifact in ${scopeLabel}. Known artifact IDs: ${validIds}` ); } } @@ -437,6 +574,49 @@ export function validateConfigRules( return warnings; } +/** + * Effective project context for a schema: the project-level `context` + * followed by that schema's `schemas..context` override, if any. The + * global field keeps applying everywhere it already did - the override is + * additive, so existing single-schema projects are unaffected. + */ +export function resolveEffectiveContext( + projectConfig: ProjectConfig | null, + schemaName: string +): string | undefined { + const globalContext = projectConfig?.context?.trim() || undefined; + const scopedContext = projectConfig?.schemas?.[schemaName]?.context?.trim() || undefined; + + if (globalContext && scopedContext) { + return `${globalContext}\n\n${scopedContext}`; + } + return globalContext ?? scopedContext; +} + +/** + * Effective rules for one artifact under a schema: the project-level + * `rules.` entries followed by that schema's + * `schemas..rules.` entries, if any. Additive for the same + * reason as `resolveEffectiveContext` - the global map still means "applies + * to this artifact everywhere". + */ +export function resolveEffectiveRules( + projectConfig: ProjectConfig | null, + schemaName: string, + artifactId: string +): string[] | undefined { + const globalRules = + projectConfig?.rules && Object.hasOwn(projectConfig.rules, artifactId) + ? projectConfig.rules[artifactId] + : undefined; + const scopedRules = projectConfig?.schemas?.[schemaName]?.rules; + const scopedRulesForArtifact = + scopedRules && Object.hasOwn(scopedRules, artifactId) ? scopedRules[artifactId] : undefined; + + const combined = [...(globalRules ?? []), ...(scopedRulesForArtifact ?? [])]; + return combined.length > 0 ? combined : undefined; +} + /** * Suggest valid schema names when user provides invalid schema. * Uses fuzzy matching to find similar names. diff --git a/test/core/artifact-graph/instruction-loader.test.ts b/test/core/artifact-graph/instruction-loader.test.ts index ce3e153255..7b56e78c14 100644 --- a/test/core/artifact-graph/instruction-loader.test.ts +++ b/test/core/artifact-graph/instruction-loader.test.ts @@ -532,6 +532,201 @@ rules: }); }); + describe('multi-schema projects (schema-scoped context/rules)', () => { + // A project-local schema that reuses the 'proposal' artifact id from + // the built-in spec-driven schema - the common case from the bug + // report, since 'proposal' is part of the default workflow shape. + function createCustomSchemaWithProposal(tempDir: string): void { + const schemaDir = path.join(tempDir, 'openspec', 'schemas', 'custom-schema'); + const templatesDir = path.join(schemaDir, 'templates'); + fs.mkdirSync(templatesDir, { recursive: true }); + fs.writeFileSync( + path.join(schemaDir, 'schema.yaml'), + `name: custom-schema +version: 1 +artifacts: + - id: proposal + generates: proposal.md + description: Custom proposal + template: proposal.md +` + ); + fs.writeFileSync(path.join(templatesDir, 'proposal.md'), '# Custom Proposal Template\n'); + } + + it('reproduces the reported leak: a global rule meant for one schema also applies to the other', () => { + // This is the exact scenario from the bug report: two schemas share + // the 'proposal' artifact id, and a rule intended for only the + // custom schema is added to the flat, schema-agnostic `rules` map + // because there is no scoping mechanism. + createCustomSchemaWithProposal(tempDir); + const configDir = path.join(tempDir, 'openspec'); + fs.writeFileSync( + path.join(configDir, 'config.yaml'), + `schema: spec-driven +rules: + proposal: + - Meant only for custom-schema +` + ); + + const specDrivenContext = loadChangeContext(tempDir, 'my-change', 'spec-driven'); + const specDrivenInstructions = generateInstructions(specDrivenContext, 'proposal', tempDir); + + // Documents today's (intentionally unchanged) behavior: a bare + // `rules.proposal` entry is global and applies to every schema's + // 'proposal' artifact, spec-driven included, even though the author + // only meant it for custom-schema. + expect(specDrivenInstructions.rules).toEqual(['Meant only for custom-schema']); + }); + + it('scopes a rule to only the intended schema via schemas..rules, fixing the leak', () => { + createCustomSchemaWithProposal(tempDir); + const configDir = path.join(tempDir, 'openspec'); + fs.writeFileSync( + path.join(configDir, 'config.yaml'), + `schema: spec-driven +schemas: + custom-schema: + rules: + proposal: + - Custom-schema-only rule +` + ); + + const specDrivenContext = loadChangeContext(tempDir, 'my-change', 'spec-driven'); + const specDrivenInstructions = generateInstructions(specDrivenContext, 'proposal', tempDir); + expect(specDrivenInstructions.rules).toBeUndefined(); + + const customContext = loadChangeContext(tempDir, 'my-change', 'custom-schema'); + const customInstructions = generateInstructions(customContext, 'proposal', tempDir); + expect(customInstructions.rules).toEqual(['Custom-schema-only rule']); + }); + + it('layers scoped rules on top of global rules for the same artifact', () => { + createCustomSchemaWithProposal(tempDir); + const configDir = path.join(tempDir, 'openspec'); + fs.writeFileSync( + path.join(configDir, 'config.yaml'), + `schema: spec-driven +rules: + proposal: + - Applies to every schema +schemas: + custom-schema: + rules: + proposal: + - Applies only to custom-schema +` + ); + + const specDrivenContext = loadChangeContext(tempDir, 'my-change', 'spec-driven'); + expect(generateInstructions(specDrivenContext, 'proposal', tempDir).rules).toEqual([ + 'Applies to every schema', + ]); + + const customContext = loadChangeContext(tempDir, 'my-change', 'custom-schema'); + expect(generateInstructions(customContext, 'proposal', tempDir).rules).toEqual([ + 'Applies to every schema', + 'Applies only to custom-schema', + ]); + }); + + it('layers scoped context on top of global context, scoped to just the intended schema', () => { + createCustomSchemaWithProposal(tempDir); + const configDir = path.join(tempDir, 'openspec'); + fs.writeFileSync( + path.join(configDir, 'config.yaml'), + `schema: spec-driven +context: Project-wide background +schemas: + custom-schema: + context: Custom-schema-only background +` + ); + + const specDrivenContext = loadChangeContext(tempDir, 'my-change', 'spec-driven'); + expect(generateInstructions(specDrivenContext, 'proposal', tempDir).context).toBe( + 'Project-wide background' + ); + + const customContext = loadChangeContext(tempDir, 'my-change', 'custom-schema'); + const customInstructions = generateInstructions(customContext, 'proposal', tempDir); + expect(customInstructions.context).toContain('Project-wide background'); + expect(customInstructions.context).toContain('Custom-schema-only background'); + }); + + it('validates schema-scoped rules against that schema\'s own artifacts, not the global union', () => { + createCustomSchemaWithProposal(tempDir); + const configDir = path.join(tempDir, 'openspec'); + fs.writeFileSync( + path.join(configDir, 'config.yaml'), + `schema: spec-driven +schemas: + custom-schema: + rules: + unknown-in-custom-schema-only: + - Some rule +` + ); + const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + try { + const customContext = loadChangeContext(tempDir, 'my-change', 'custom-schema'); + generateInstructions(customContext, 'proposal', tempDir); + + expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining( + 'Unknown artifact ID in rules: "unknown-in-custom-schema-only"' + ) + ); + expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining("schema 'custom-schema'") + ); + } finally { + consoleWarnSpy.mockRestore(); + } + }); + + it('warns when a schemas. entry names no available schema, instead of silently dropping it', () => { + const configDir = path.join(tempDir, 'openspec'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync( + path.join(configDir, 'config.yaml'), + `schema: spec-driven +schemas: + typo-ed-schema-name: + context: This never applies to anything + rules: + proposal: + - This never applies either +` + ); + const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + try { + const context = loadChangeContext(tempDir, 'my-change', 'spec-driven'); + const instructions = generateInstructions(context, 'proposal', tempDir); + + // The typo'd override matches no real schema, so it never affects + // the resolved instructions... + expect(instructions.context).toBeUndefined(); + expect(instructions.rules).toBeUndefined(); + + // ...but it must not fail silently - the user should learn why, + // including which schema name they probably meant. + expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining("Unknown schema 'typo-ed-schema-name' in config 'schemas'") + ); + expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining('Known schemas: spec-driven') + ); + } finally { + consoleWarnSpy.mockRestore(); + } + }); + }); + describe('validation and warnings', () => { let consoleWarnSpy: ReturnType; diff --git a/test/core/project-config.test.ts b/test/core/project-config.test.ts index 2adbdf9ad6..509f087fe3 100644 --- a/test/core/project-config.test.ts +++ b/test/core/project-config.test.ts @@ -8,6 +8,8 @@ import { readProjectConfig, validateConfigRules, suggestSchemas, + resolveEffectiveContext, + resolveEffectiveRules, } from '../../src/core/project-config.js'; describe('project-config', () => { @@ -832,6 +834,254 @@ rules: ]); }); }); + + describe('schemas field (schema-scoped context/rules overrides)', () => { + it('should parse per-schema context and rules, keyed by schema name', () => { + const configDir = path.join(tempDir, 'openspec'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync( + path.join(configDir, 'config.yaml'), + `schema: spec-driven +schemas: + custom-schema: + context: Custom schema background + rules: + proposal: + - Custom schema rule +` + ); + + const config = readProjectConfig(tempDir); + + expect(config?.schemas).toEqual({ + 'custom-schema': { + context: 'Custom schema background', + rules: { proposal: ['Custom schema rule'] }, + }, + }); + expect(consoleWarnSpy).not.toHaveBeenCalled(); + }); + + it('should support multiple scoped schemas independently', () => { + const configDir = path.join(tempDir, 'openspec'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync( + path.join(configDir, 'config.yaml'), + `schema: spec-driven +schemas: + schema-a: + rules: + proposal: + - Rule for schema-a + schema-b: + context: Background for schema-b +` + ); + + const config = readProjectConfig(tempDir); + + expect(config?.schemas).toEqual({ + 'schema-a': { rules: { proposal: ['Rule for schema-a'] } }, + 'schema-b': { context: 'Background for schema-b' }, + }); + }); + + it('should return undefined for schemas when absent', () => { + fs.mkdirSync(path.join(tempDir, 'openspec'), { recursive: true }); + fs.writeFileSync(path.join(tempDir, 'openspec', 'config.yaml'), 'schema: spec-driven\n'); + + expect(readProjectConfig(tempDir)?.schemas).toBeUndefined(); + }); + + it('should warn and drop schemas when not an object', () => { + const configDir = path.join(tempDir, 'openspec'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync( + path.join(configDir, 'config.yaml'), + 'schema: spec-driven\nschemas: ["not", "an", "object"]\n' + ); + + const config = readProjectConfig(tempDir); + + expect(config?.schemas).toBeUndefined(); + expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining("Invalid 'schemas' field") + ); + }); + + it('should warn and drop a single schema entry that is not an object, keeping others', () => { + const configDir = path.join(tempDir, 'openspec'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync( + path.join(configDir, 'config.yaml'), + `schema: spec-driven +schemas: + broken-schema: "not an object" + good-schema: + context: Fine +` + ); + + const config = readProjectConfig(tempDir); + + expect(config?.schemas).toEqual({ 'good-schema': { context: 'Fine' } }); + expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining("Invalid 'schemas.broken-schema' field") + ); + }); + + it('should warn about unknown fields inside a schema override', () => { + const configDir = path.join(tempDir, 'openspec'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync( + path.join(configDir, 'config.yaml'), + `schema: spec-driven +schemas: + custom-schema: + unknownField: nope + context: Fine +` + ); + + const config = readProjectConfig(tempDir); + + expect(config?.schemas?.['custom-schema']).toEqual({ context: 'Fine' }); + expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining("Unknown field(s) in 'schemas.custom-schema': unknownField") + ); + }); + + it('should enforce the context size limit per scoped schema', () => { + const configDir = path.join(tempDir, 'openspec'); + fs.mkdirSync(configDir, { recursive: true }); + const largeContext = 'a'.repeat(51 * 1024); + fs.writeFileSync( + path.join(configDir, 'config.yaml'), + `schema: spec-driven\nschemas:\n custom-schema:\n context: "${largeContext}"\n` + ); + + const config = readProjectConfig(tempDir); + + expect(config?.schemas).toBeUndefined(); + expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining("Context too large in 'schemas.custom-schema.context'") + ); + }); + + it('should filter out empty string rules within a scoped schema', () => { + const configDir = path.join(tempDir, 'openspec'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync( + path.join(configDir, 'config.yaml'), + `schema: spec-driven +schemas: + custom-schema: + rules: + proposal: + - "" + - Real rule +` + ); + + const config = readProjectConfig(tempDir); + + expect(config?.schemas?.['custom-schema']?.rules).toEqual({ proposal: ['Real rule'] }); + expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining("Some rules for 'proposal' in 'schemas.custom-schema.rules'") + ); + }); + + it('should preserve prototype-named schema names as inert data', () => { + const configDir = path.join(tempDir, 'openspec'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync( + path.join(configDir, 'config.yaml'), + `schema: spec-driven +schemas: + __proto__: + context: Prototype schema context +` + ); + + const schemas = readProjectConfig(tempDir)?.schemas; + + expect(Object.getPrototypeOf(schemas)).toBeNull(); + expect(Object.hasOwn(schemas!, '__proto__')).toBe(true); + expect(schemas?.__proto__).toEqual({ context: 'Prototype schema context' }); + }); + }); + }); + + describe('resolveEffectiveContext', () => { + it('returns the global context when no schema override exists', () => { + const config = { schema: 'spec-driven', context: 'Global background' }; + expect(resolveEffectiveContext(config, 'spec-driven')).toBe('Global background'); + }); + + it('returns only the scoped context when no global context exists', () => { + const config = { + schema: 'spec-driven', + schemas: { 'custom-schema': { context: 'Scoped background' } }, + }; + expect(resolveEffectiveContext(config, 'custom-schema')).toBe('Scoped background'); + }); + + it('combines global and scoped context for the matching schema', () => { + const config = { + schema: 'spec-driven', + context: 'Global background', + schemas: { 'custom-schema': { context: 'Scoped background' } }, + }; + expect(resolveEffectiveContext(config, 'custom-schema')).toBe( + 'Global background\n\nScoped background' + ); + }); + + it('does not leak a scoped context into a different schema', () => { + const config = { + schema: 'spec-driven', + schemas: { 'custom-schema': { context: 'Scoped background' } }, + }; + expect(resolveEffectiveContext(config, 'spec-driven')).toBeUndefined(); + }); + + it('returns undefined when there is no context at all', () => { + expect(resolveEffectiveContext(null, 'spec-driven')).toBeUndefined(); + expect(resolveEffectiveContext({ schema: 'spec-driven' }, 'spec-driven')).toBeUndefined(); + }); + }); + + describe('resolveEffectiveRules', () => { + it('returns global rules for an artifact when no schema override exists', () => { + const config = { schema: 'spec-driven', rules: { proposal: ['Global rule'] } }; + expect(resolveEffectiveRules(config, 'spec-driven', 'proposal')).toEqual(['Global rule']); + }); + + it('does not leak a scoped rule into a different schema', () => { + const config = { + schema: 'spec-driven', + schemas: { 'custom-schema': { rules: { proposal: ['Scoped rule'] } } }, + }; + expect(resolveEffectiveRules(config, 'spec-driven', 'proposal')).toBeUndefined(); + expect(resolveEffectiveRules(config, 'custom-schema', 'proposal')).toEqual(['Scoped rule']); + }); + + it('appends scoped rules after global rules for the same artifact', () => { + const config = { + schema: 'spec-driven', + rules: { proposal: ['Global rule'] }, + schemas: { 'custom-schema': { rules: { proposal: ['Scoped rule'] } } }, + }; + expect(resolveEffectiveRules(config, 'custom-schema', 'proposal')).toEqual([ + 'Global rule', + 'Scoped rule', + ]); + }); + + it('returns undefined when neither global nor scoped rules exist for the artifact', () => { + const config = { schema: 'spec-driven', rules: { specs: ['Other artifact rule'] } }; + expect(resolveEffectiveRules(config, 'spec-driven', 'proposal')).toBeUndefined(); + }); }); describe('loadOperationInputs', () => { @@ -942,6 +1192,18 @@ rules: expect(warnings).toEqual([]); }); + + it('should word the warning for a specific schema when schemaName is passed', () => { + const rules = { 'unknown-artifact': ['Rule 1'] }; + const validIds = new Set(['proposal', 'specs']); + + const warnings = validateConfigRules(rules, validIds, 'custom-schema'); + + expect(warnings).toHaveLength(1); + expect(warnings[0]).toContain('Unknown artifact ID in rules: "unknown-artifact"'); + expect(warnings[0]).toContain("schema 'custom-schema'"); + expect(warnings[0]).not.toContain('any available schema'); + }); }); describe('suggestSchemas', () => {