diff --git a/src/cli/index.ts b/src/cli/index.ts index 4a4c5311..780ba1d8 100644 --- a/src/cli/index.ts +++ b/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); diff --git a/src/core/archive.ts b/src/core/archive.ts index 051f908e..bf94ee59 100644 --- a/src/core/archive.ts +++ b/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 { + async execute( + changeName?: string, + options: { yes?: boolean; skipSpecs?: boolean; noValidate?: boolean; validate?: boolean } = {} + ): Promise { 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]; } -} \ No newline at end of file +} diff --git a/src/core/validation/validator.ts b/src/core/validation/validator.ts index 24b3d415..c9c45ba4 100644 --- a/src/core/validation/validator.ts +++ b/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 { diff --git a/test/core/archive.test.ts b/test/core/archive.test.ts index 5d590a5a..d950d219 100644 --- a/test/core/archive.test.ts +++ b/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; @@ -636,4 +677,4 @@ E1 updated`); await expect(fs.access(changeDir)).resolves.not.toThrow(); }); }); -}); \ No newline at end of file +}); diff --git a/test/core/validation.test.ts b/test/core/validation.test.ts index 96c07d7e..71e63535 100644 --- a/test/core/validation.test.ts +++ b/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); + }); + }); }); \ No newline at end of file