Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 20 additions & 2 deletions src/core/validation/validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down
86 changes: 86 additions & 0 deletions test/core/validation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down