diff --git a/src/core/validation/validator.ts b/src/core/validation/validator.ts index 37b43a37d..47071ed47 100644 --- a/src/core/validation/validator.ts +++ b/src/core/validation/validator.ts @@ -165,7 +165,7 @@ export class Validator { if (!requirementText) { issues.push({ level: 'ERROR', path: entryPath, message: `ADDED "${block.name}" is missing requirement text` }); } else if (!this.containsShallOrMust(requirementText)) { - issues.push({ level: 'ERROR', path: entryPath, message: `ADDED "${block.name}" must contain SHALL or MUST` }); + issues.push({ level: 'ERROR', path: entryPath, message: this.buildMissingShallOrMustMessage('ADDED', block.name) }); } const scenarioCount = this.countScenarios(block.raw); if (scenarioCount < 1) { @@ -186,7 +186,7 @@ export class Validator { if (!requirementText) { issues.push({ level: 'ERROR', path: entryPath, message: `MODIFIED "${block.name}" is missing requirement text` }); } else if (!this.containsShallOrMust(requirementText)) { - issues.push({ level: 'ERROR', path: entryPath, message: `MODIFIED "${block.name}" must contain SHALL or MUST` }); + issues.push({ level: 'ERROR', path: entryPath, message: this.buildMissingShallOrMustMessage('MODIFIED', block.name) }); } const scenarioCount = this.countScenarios(block.raw); if (scenarioCount < 1) { @@ -444,6 +444,24 @@ export class Validator { return /\b(SHALL|MUST)\b/.test(text); } + /** + * Build an error message for a requirement block whose body lacks SHALL/MUST. + * + * When the SHALL/MUST keyword already appears in the requirement header (e.g. + * `### Requirement: The system SHALL ...`) the original generic error + * ("must contain SHALL or MUST") is confusing because the keyword is visibly + * present in the spec. Per the OpenSpec conventions the keyword has to live + * on the requirement body line (the line right after the header), so we point + * the author at that exact fix when the keyword is found in the header only. + */ + private buildMissingShallOrMustMessage(action: 'ADDED' | 'MODIFIED', blockName: string): string { + const base = `${action} "${blockName}" must contain SHALL or MUST`; + if (this.containsShallOrMust(blockName)) { + return `${base} in the requirement body, not only in the header. Move the SHALL/MUST statement to the line immediately after the "### Requirement: ..." header.`; + } + return base; + } + private countScenarios(blockRaw: string): number { const matches = blockRaw.match(/^####\s+/gm); return matches ? matches.length : 0; diff --git a/test/core/validation.test.ts b/test/core/validation.test.ts index 972815e51..72ebc2aba 100644 --- a/test/core/validation.test.ts +++ b/test/core/validation.test.ts @@ -535,6 +535,92 @@ The system will log all events. expect(report.issues.some(i => i.message.includes('must contain SHALL or MUST'))).toBe(true); }); + it('should hint the author when ADDED requirement only has SHALL/MUST in the header', async () => { + const changeDir = path.join(testDir, 'test-change-shall-in-header-added'); + const specsDir = path.join(changeDir, 'specs', 'test-spec'); + await fs.mkdir(specsDir, { recursive: true }); + + const deltaSpec = `# Test Spec + +## ADDED Requirements + +### Requirement: The system SHALL log all errors +Error handling logic goes here. + +#### Scenario: Error occurs +**Given** an error +**When** it occurs +**Then** it is logged`; + + const specPath = path.join(specsDir, 'spec.md'); + await fs.writeFile(specPath, deltaSpec); + + const validator = new Validator(true); + const report = await validator.validateChangeDeltaSpecs(changeDir); + + expect(report.valid).toBe(false); + const shallMessage = report.issues.find(i => i.message.includes('must contain SHALL or MUST')); + expect(shallMessage?.message).toContain('not only in the header'); + expect(shallMessage?.message).toContain('### Requirement:'); + }); + + it('should hint the author when MODIFIED requirement only has SHALL/MUST in the header', async () => { + const changeDir = path.join(testDir, 'test-change-shall-in-header-modified'); + const specsDir = path.join(changeDir, 'specs', 'test-spec'); + await fs.mkdir(specsDir, { recursive: true }); + + const deltaSpec = `# Test Spec + +## MODIFIED Requirements + +### Requirement: The system MUST validate user input +Please describe how validation should work here. + +#### Scenario: Invalid input +**Given** invalid input +**When** validation runs +**Then** an error surfaces`; + + const specPath = path.join(specsDir, 'spec.md'); + await fs.writeFile(specPath, deltaSpec); + + const validator = new Validator(true); + const report = await validator.validateChangeDeltaSpecs(changeDir); + + expect(report.valid).toBe(false); + const shallMessage = report.issues.find(i => i.message.includes('must contain SHALL or MUST')); + expect(shallMessage?.message).toContain('not only in the header'); + expect(shallMessage?.message).toContain('### Requirement:'); + }); + + it('should keep the generic SHALL/MUST error when neither header nor body contain the keyword', async () => { + const changeDir = path.join(testDir, 'test-change-shall-nowhere'); + const specsDir = path.join(changeDir, 'specs', 'test-spec'); + await fs.mkdir(specsDir, { recursive: true }); + + const deltaSpec = `# Test Spec + +## ADDED Requirements + +### Requirement: Logging Feature +The system will log all events. + +#### Scenario: Event occurs +**Given** an event +**When** it occurs +**Then** it is logged`; + + const specPath = path.join(specsDir, 'spec.md'); + await fs.writeFile(specPath, deltaSpec); + + const validator = new Validator(true); + const report = await validator.validateChangeDeltaSpecs(changeDir); + + expect(report.valid).toBe(false); + const shallMessage = report.issues.find(i => i.message.includes('must contain SHALL or MUST')); + expect(shallMessage?.message).not.toContain('not only in the header'); + }); + it('should handle requirements without metadata fields', async () => { const changeDir = path.join(testDir, 'test-change-4'); const specsDir = path.join(changeDir, 'specs', 'test-spec');