fix: honor --no-validate and ignore metadata during archive validation (#190)

* fix: skip metadata when validating requirement SHALL/MUST keywords

Fixes validation incorrectly checking metadata lines instead of requirement text.

The extractRequirementText() function was returning the first non-empty line
after the requirement header, which was often metadata like **ID**: REQ-001
instead of the actual requirement statement.

Changes:
- Updated extractRequirementText() to skip lines matching **Key**: Value pattern
- Skip blank lines between header and requirement text
- Return first substantive text line for SHALL/MUST validation

Added comprehensive tests for:
- Requirements with metadata before SHALL/MUST text
- Requirements with SHALL in text but not header
- Requirements correctly failing without SHALL/MUST
- Requirements without metadata fields

All 20 validation tests pass.

Fixes #159

* Respect --no-validate flag while archiving

Author: TabishB

src/cli/index.ts

@@ -187,7 +187,7 @@ program
   .option('-y, --yes', 'Skip confirmation prompts')
   .option('--skip-specs', 'Skip spec update operations (useful for infrastructure, tooling, or doc-only changes)')
   .option('--no-validate', 'Skip validation (not recommended, requires confirmation)')
-  .action(async (changeName?: string, options?: { yes?: boolean; skipSpecs?: boolean; noValidate?: boolean }) => {
+  .action(async (changeName?: string, options?: { yes?: boolean; skipSpecs?: boolean; noValidate?: boolean; validate?: boolean }) => {
     try {
       const archiveCommand = new ArchiveCommand();
       await archiveCommand.execute(changeName, options);

src/core/archive.ts

@@ -19,7 +19,10 @@ interface SpecUpdate {
 }
 
 export class ArchiveCommand {
-  async execute(changeName?: string, options: { yes?: boolean; skipSpecs?: boolean; noValidate?: boolean } = {}): Promise<void> {
+  async execute(
+    changeName?: string,
+    options: { yes?: boolean; skipSpecs?: boolean; noValidate?: boolean; validate?: boolean } = {}
+  ): Promise<void> {
     const targetPath = '.';
     const changesDir = path.join(targetPath, 'openspec', 'changes');
     const archiveDir = path.join(changesDir, 'archive');
@@ -54,8 +57,10 @@ export class ArchiveCommand {
       throw new Error(`Change '${changeName}' not found.`);
     }
 
+    const skipValidation = options.validate === false || options.noValidate === true;
+
     // Validate specs and change before archiving
-    if (!options.noValidate) {
+    if (!skipValidation) {
       const validator = new Validator();
       let hasValidationErrors = false;
 
@@ -201,7 +206,7 @@ export class ArchiveCommand {
           let totals = { added: 0, modified: 0, removed: 0, renamed: 0 };
           for (const p of prepared) {
             const specName = path.basename(path.dirname(p.update.target));
-            if (!options.noValidate) {
+            if (!skipValidation) {
               const report = await new Validator().validateSpecContent(specName, p.rebuilt);
               if (!report.valid) {
                 console.log(chalk.red(`\nValidation errors in rebuilt spec for ${specName} (will not write changes):`));
@@ -598,4 +603,4 @@ export class ArchiveCommand {
     // Returns date in YYYY-MM-DD format
     return new Date().toISOString().split('T')[0];
   }
-}
+}

src/core/validation/validator.ts

@@ -375,16 +375,30 @@ export class Validator {
 
   private extractRequirementText(blockRaw: string): string | undefined {
     const lines = blockRaw.split('\n');
-    // Skip header
+    // Skip header line (index 0)
     let i = 1;
-    const bodyLines: string[] = [];
+
+    // Find the first substantial text line, skipping metadata and blank lines
     for (; i < lines.length; i++) {
       const line = lines[i];
-      if (/^####\s+/.test(line)) break; // scenarios start
-      bodyLines.push(line);
+
+      // Stop at scenario headers
+      if (/^####\s+/.test(line)) break;
+
+      const trimmed = line.trim();
+
+      // Skip blank lines
+      if (trimmed.length === 0) continue;
+
+      // Skip metadata lines (lines starting with ** like **ID**, **Priority**, etc.)
+      if (/^\*\*[^*]+\*\*:/.test(trimmed)) continue;
+
+      // Found first non-metadata, non-blank line - this is the requirement text
+      return trimmed;
     }
-    const text = bodyLines.join('\n').split('\n').map(l => l.trim()).find(l => l.length > 0);
-    return text;
+
+    // No requirement text found
+    return undefined;
   }
 
   private containsShallOrMust(text: string): boolean {

test/core/archive.test.ts

@@ -1,5 +1,6 @@
 import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
 import { ArchiveCommand } from '../../src/core/archive.js';
+import { Validator } from '../../src/core/validation/validator.js';
 import { promises as fs } from 'fs';
 import path from 'path';
 import os from 'os';
@@ -215,6 +216,46 @@ Then expected result happens`;
       expect(archives[0]).toMatch(new RegExp(`\\d{4}-\\d{2}-\\d{2}-${changeName}`));
     });
 
+    it('should skip validation when commander sets validate to false (--no-validate)', async () => {
+      const changeName = 'skip-validation-flag';
+      const changeDir = path.join(tempDir, 'openspec', 'changes', changeName);
+      const changeSpecDir = path.join(changeDir, 'specs', 'unstable-capability');
+      await fs.mkdir(changeSpecDir, { recursive: true });
+
+      const deltaSpec = `# Unstable Capability
+
+## ADDED Requirements
+
+### Requirement: Logging Feature
+**ID**: REQ-LOG-001
+
+The system will log all events.
+
+#### Scenario: Event recorded
+- **WHEN** an event occurs
+- **THEN** it is captured`;
+      await fs.writeFile(path.join(changeSpecDir, 'spec.md'), deltaSpec);
+      await fs.writeFile(path.join(changeDir, 'tasks.md'), '- [x] Task 1\n');
+
+      const deltaSpy = vi.spyOn(Validator.prototype, 'validateChangeDeltaSpecs');
+      const specContentSpy = vi.spyOn(Validator.prototype, 'validateSpecContent');
+
+      try {
+        await archiveCommand.execute(changeName, { yes: true, skipSpecs: true, validate: false });
+
+        expect(deltaSpy).not.toHaveBeenCalled();
+        expect(specContentSpy).not.toHaveBeenCalled();
+
+        const archiveDir = path.join(tempDir, 'openspec', 'changes', 'archive');
+        const archives = await fs.readdir(archiveDir);
+        expect(archives.length).toBe(1);
+        expect(archives[0]).toMatch(new RegExp(`\\d{4}-\\d{2}-\\d{2}-${changeName}`));
+      } finally {
+        deltaSpy.mockRestore();
+        specContentSpy.mockRestore();
+      }
+    });
+
     it('should proceed with archive when user declines spec updates', async () => {
       const { confirm } = await import('@inquirer/prompts');
       const mockConfirm = confirm as unknown as ReturnType<typeof vi.fn>;
@@ -636,4 +677,4 @@ E1 updated`);
       await expect(fs.access(changeDir)).resolves.not.toThrow();
     });
   });
-});
+});

test/core/validation.test.ts

@@ -306,10 +306,10 @@ Then result`;
 
       const specPath = path.join(testDir, 'spec.md');
       await fs.writeFile(specPath, specContent);
-      
+
       const validator = new Validator(true); // strict mode
       const report = await validator.validateSpec(specPath);
-      
+
       expect(report.valid).toBe(false); // Should fail due to brief overview warning
     });
 
@@ -330,12 +330,131 @@ Then result`;
 
       const specPath = path.join(testDir, 'spec.md');
       await fs.writeFile(specPath, specContent);
-      
+
       const validator = new Validator(false); // non-strict mode
       const report = await validator.validateSpec(specPath);
-      
+
       expect(report.valid).toBe(true); // Should pass despite warnings
       expect(report.summary.warnings).toBeGreaterThan(0);
     });
   });
+
+  describe('validateChangeDeltaSpecs with metadata', () => {
+    it('should validate requirement with metadata before SHALL/MUST text', async () => {
+      const changeDir = path.join(testDir, 'test-change');
+      const specsDir = path.join(changeDir, 'specs', 'test-spec');
+      await fs.mkdir(specsDir, { recursive: true });
+
+      const deltaSpec = `# Test Spec
+
+## ADDED Requirements
+
+### Requirement: Circuit Breaker State Management SHALL be implemented
+**ID**: REQ-CB-001
+**Priority**: P1 (High)
+
+The system MUST implement a circuit breaker with three states.
+
+#### Scenario: Normal operation
+**Given** the circuit breaker is in CLOSED state
+**When** a request is made
+**Then** the request is executed normally`;
+
+      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(true);
+      expect(report.summary.errors).toBe(0);
+    });
+
+    it('should validate requirement with SHALL in text but not in header', async () => {
+      const changeDir = path.join(testDir, 'test-change-2');
+      const specsDir = path.join(changeDir, 'specs', 'test-spec');
+      await fs.mkdir(specsDir, { recursive: true });
+
+      const deltaSpec = `# Test Spec
+
+## ADDED Requirements
+
+### Requirement: Error Handling
+**ID**: REQ-ERR-001
+**Priority**: P2
+
+The system SHALL handle all errors gracefully.
+
+#### Scenario: Error occurs
+**Given** an error condition
+**When** an error occurs
+**Then** the error is logged and user is notified`;
+
+      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(true);
+      expect(report.summary.errors).toBe(0);
+    });
+
+    it('should fail when requirement text lacks SHALL/MUST', async () => {
+      const changeDir = path.join(testDir, 'test-change-3');
+      const specsDir = path.join(changeDir, 'specs', 'test-spec');
+      await fs.mkdir(specsDir, { recursive: true });
+
+      const deltaSpec = `# Test Spec
+
+## ADDED Requirements
+
+### Requirement: Logging Feature
+**ID**: REQ-LOG-001
+
+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);
+      expect(report.summary.errors).toBeGreaterThan(0);
+      expect(report.issues.some(i => i.message.includes('must contain SHALL or MUST'))).toBe(true);
+    });
+
+    it('should handle requirements without metadata fields', async () => {
+      const changeDir = path.join(testDir, 'test-change-4');
+      const specsDir = path.join(changeDir, 'specs', 'test-spec');
+      await fs.mkdir(specsDir, { recursive: true });
+
+      const deltaSpec = `# Test Spec
+
+## ADDED Requirements
+
+### Requirement: Simple Feature
+The system SHALL implement this feature.
+
+#### Scenario: Basic usage
+**Given** a condition
+**When** an action occurs
+**Then** a result happens`;
+
+      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(true);
+      expect(report.summary.errors).toBe(0);
+    });
+  });
 });