fix(validator): hint when SHALL/MUST appears only in requirement header (Fission-AI#1135)

When a change delta has a requirement whose body is missing SHALL/MUST but
whose header (the text after `### Requirement:`) already contains the
keyword, the validator emitted the generic error "must contain SHALL or
MUST". Authors then re-read the spec, see SHALL right there in the header,
and have no idea what the validator wants.

Per the OpenSpec conventions the keyword has to live on the requirement
body line (the line immediately after the header). When the keyword is
present in the header only, append guidance explaining exactly where to
move it. The fix is scoped to the two `validateChangeDeltaSpecs` call
sites (ADDED + MODIFIED) so behaviour for requirements that lack the
keyword everywhere stays unchanged.

Adds three vitest cases under `test/core/validation.test.ts`:
- ADDED block with header-only SHALL → enriched hint
- MODIFIED block with header-only MUST → enriched hint
- Neither header nor body contain SHALL/MUST → generic message preserved

Verified by reproducing the spec from Fission-AI#356, running
`openspec validate <change>` against the rebuilt CLI, and confirming the
new diagnostic guides the author to the fix. Reverting `validator.ts`
makes the two enriched-hint cases fail, so the tests guard the
regression.

Fixes Fission-AI#356

Co-authored-by: Pluviobyte <Pluviobyte@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Tabish Bidiwale <30385142+TabishB@users.noreply.github.com>

Author: 3 people

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;

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');