Skip to content

Commit bac6ab7

Browse files
proposal: fix metadata field validation bug
Add OpenSpec change proposal and test coverage for requirement validation bug where metadata fields (**ID**, **Priority**, etc.) were incorrectly checked for SHALL/MUST keywords instead of the requirement description. The bug caused false validation failures on valid requirements that included metadata fields before the normative description text. Following TDD approach: - Added failing test case for requirements with metadata fields - Test fails with buggy code (pre-c782462) - Test passes with fixed code (commit c782462) - Fix skips metadata field lines when extracting requirement text 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
1 parent fb7ff52 commit bac6ab7

File tree

4 files changed

+134
-0
lines changed

4 files changed

+134
-0
lines changed
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
# Proposal: Fix Metadata Field Validation Bug
2+
3+
## Why
4+
5+
The requirement SHALL/MUST validation incorrectly checks metadata fields instead of the requirement description, causing false failures on valid requirements.
6+
7+
**The Bug**: When a requirement includes metadata fields (like `**ID**: REQ-001` or `**Priority**: P1`) immediately after the title, the validator extracts the first non-empty line as the "requirement text". This returns a metadata field like `**ID**: REQ-001`, which doesn't contain SHALL/MUST, causing validation to fail even when the actual requirement description contains proper normative keywords.
8+
9+
**Example that fails incorrectly**:
10+
```markdown
11+
### Requirement: File Serving
12+
**ID**: REQ-FILE-001
13+
**Priority**: P1
14+
15+
The system MUST serve static files from the root directory.
16+
```
17+
18+
The validator checks `**ID**: REQ-FILE-001` for SHALL/MUST instead of checking `The system MUST serve...`, so it fails.
19+
20+
**Root Cause**: The `extractRequirementText()` method in validator.ts collected all lines after the requirement header, joined them, and returned the first non-empty line. It didn't skip metadata field lines (matching pattern `/^\*\*[^*]+\*\*:/`).
21+
22+
**Impact**: Users cannot use metadata fields in requirements without triggering false validation failures, blocking adoption of structured requirement metadata.
23+
24+
## What Changes
25+
26+
- **Fix** requirement text extraction to skip metadata field lines before finding the normative description
27+
- **Add** comprehensive test coverage for requirements with metadata fields
28+
29+
## Impact
30+
31+
- **Modified specs**: cli-validate
32+
- **Bug fix**: Resolves false positive validation failures
33+
- **No breaking changes**: Existing valid requirements continue to pass; previously failing valid requirements now pass correctly
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
# cli-validate Specification
2+
3+
## MODIFIED Requirements
4+
5+
### Requirement: Metadata field extraction
6+
7+
The validator SHALL skip metadata field lines when extracting requirement description text for SHALL/MUST validation.
8+
9+
#### Scenario: Requirement with metadata fields before description
10+
11+
- **GIVEN** a requirement with metadata fields (`**ID**:`, `**Priority**:`, etc.) between the title and description
12+
- **WHEN** validating for SHALL/MUST keywords
13+
- **THEN** the validator SHALL skip all lines matching `/^\*\*[^*]+\*\*:/` pattern
14+
- **AND** SHALL extract the first substantial non-metadata line as the requirement description
15+
- **AND** SHALL check that description (not metadata) for SHALL or MUST keywords
16+
17+
#### Scenario: Requirement without metadata fields
18+
19+
- **GIVEN** a requirement with no metadata fields
20+
- **WHEN** validating for SHALL/MUST keywords
21+
- **THEN** the validator SHALL extract the first non-empty line after the title
22+
- **AND** SHALL check that line for SHALL or MUST keywords
23+
24+
#### Scenario: Requirement with blank lines before description
25+
26+
- **GIVEN** a requirement with blank lines between metadata/title and description
27+
- **WHEN** validating for SHALL/MUST keywords
28+
- **THEN** the validator SHALL skip blank lines
29+
- **AND** SHALL extract the first substantial text line as the requirement description
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
# Tasks
2+
3+
## Test-Driven Development (Red-Green)
4+
5+
1. **✅ Write failing test (RED)**
6+
- Added test case: "should pass when requirement has metadata fields before description with SHALL/MUST"
7+
- Test creates requirement with `**ID**` and `**Priority**` metadata before normative description
8+
- Test expects validation to pass (description contains MUST)
9+
- With buggy code (pre-c782462): Test FAILS (validator checks metadata field instead of description)
10+
11+
2. **✅ Verify test fails for correct reason**
12+
- Confirmed test fails with buggy code
13+
- Error: `expect(report.valid).toBe(true)` but got `false`
14+
- Root cause: `extractRequirementText()` returns `**ID**: REQ-FILE-001` which lacks SHALL/MUST
15+
16+
3. **✅ Implement fix (GREEN)**
17+
- Modified `extractRequirementText()` in src/core/validation/validator.ts
18+
- Changed from collecting all lines and finding first non-empty
19+
- To: iterating through lines, skipping blanks and metadata fields
20+
- Returns first line that is NOT blank and NOT metadata pattern `/^\*\*[^*]+\*\*:/`
21+
- Fix implemented in commit c782462
22+
23+
4. **✅ Verify test passes without modification**
24+
- Test now passes with fixed code
25+
- No changes to test needed - acceptance criteria met
26+
- Validator correctly skips metadata and checks actual description
27+
28+
## Validation
29+
30+
5. **Run full test suite**
31+
- Execute `pnpm test` to ensure no regressions
32+
- All existing tests should pass
33+
- New test provides coverage for metadata field bug
34+
35+
6. **Validate proposal**
36+
- Run `openspec validate fix-metadata-field-validation-bug --strict`
37+
- Ensure proposal structure is correct
38+
- Confirm all sections present and valid

test/core/validation.test.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -485,5 +485,39 @@ The system MUST support mixed case delta headers.
485485
expect(report.summary.warnings).toBe(0);
486486
expect(report.summary.info).toBe(0);
487487
});
488+
489+
it('should pass when requirement has metadata fields before description with SHALL/MUST', async () => {
490+
const changeDir = path.join(testDir, 'test-change-with-metadata');
491+
const specsDir = path.join(changeDir, 'specs', 'test-spec');
492+
await fs.mkdir(specsDir, { recursive: true });
493+
494+
// This is the exact pattern that was failing before the metadata fix
495+
// The bug: old code would check **ID** line for SHALL/MUST instead of the description
496+
const deltaSpec = `# Test Spec
497+
498+
## ADDED Requirements
499+
500+
### Requirement: File Serving
501+
**ID**: REQ-FILE-001
502+
**Priority**: P1
503+
504+
The system MUST serve static files from the root directory.
505+
506+
#### Scenario: File is requested
507+
**Given** a static file exists
508+
**When** the file is requested
509+
**Then** the system SHALL serve the file successfully`;
510+
511+
const specPath = path.join(specsDir, 'spec.md');
512+
await fs.writeFile(specPath, deltaSpec);
513+
514+
const validator = new Validator(true);
515+
const report = await validator.validateChangeDeltaSpecs(changeDir);
516+
517+
// This should PASS because the description (not metadata) contains MUST
518+
// Before fix c782462, this would FAIL because it checked the **ID** line
519+
expect(report.valid).toBe(true);
520+
expect(report.summary.errors).toBe(0);
521+
});
488522
});
489523
});

0 commit comments

Comments
 (0)