fix(validator): hint main spec header-only SHALL/MUST

[Pluviobyte]

Summary

Fixes #1156.

OpenSpec 1.4.0 already gives a targeted SHALL/MUST hint for change delta specs when the keyword appears only in the ### Requirement: header. Main specs under openspec/specs/ still went through the generic schema error, so authors saw only Requirement must contain SHALL or MUST keyword even though the keyword was visible in the heading.

This PR reuses the same diagnostic shape for main specs: when a requirement body lacks SHALL/MUST but the corresponding requirement heading contains it, the validator points the author to move the normative statement to the requirement body line. Cases where the keyword is missing everywhere keep the existing generic message.

Changes

• Convert main spec zod errors with access to the original markdown content.
• Recover the matching requirement heading for requirements.N.text errors while ignoring fenced code blocks.
• Add a regression test for a main spec requirement with SHALL only in the header.

Verification

• pnpm exec vitest run test/core/validation.test.ts
• pnpm run build
• pnpm run lint
• pnpm test (89 files, 1656 tests)
• Reproduced openspec validate --all --json on a temporary main spec and confirmed the emitted error includes not only in the header instead of the generic-only message.

Summary by CodeRabbit

• Bug Fixes

  • Improved validation error messages for requirements missing SHALL/MUST, providing requirement-specific guidance instead of generic errors
  • Clarified hints when SHALL/MUST appears only in a requirement header, instructing users to move the keyword into the requirement body

• Tests

  • Added/updated tests covering cases where SHALL/MUST appears only in requirement headers to ensure corrected messaging

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c562a31d-20aa-49e8-8c37-c76451ebff75

📥 Commits

Reviewing files that changed from the base of the PR and between f72dc6d and 035622b.

📒 Files selected for processing (2)

• src/core/validation/validator.ts
• test/core/validation.test.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/core/validation/validator.ts

---

📝 Walkthrough

Walkthrough

Adds a spec-specific Zod error converter that extracts requirement names from main-spec Markdown, maps Zod error paths to requirement indices, and produces targeted SHALL/MUST hints; wires this converter into validateSpec/validateSpecContent and updates the message builder and tests.

Changes

Main-spec SHALL/MUST validation hints

Layer / File(s)	Summary
Spec validation error conversion and markdown parsing src/core/validation/validator.ts	Adds convertSpecZodErrors and helpers (getRequirementIndexFromPath, extractMainSpecRequirementNames) that map Zod error paths to requirement indices and extract requirement names from the main "Requirements" section while skipping fenced code blocks.
Validator method integration src/core/validation/validator.ts	Updates validateSpec and validateSpecContent to use convertSpecZodErrors instead of generic convertZodErrors, enabling requirement-body-specific error messaging for main specs.
Error message builder enhancement src/core/validation/validator.ts	Extends buildMissingShallOrMustMessage to accept 'Requirement' and to change guidance when SHALL/MUST already appears in the requirement header (advise moving it to the body line).
Test validation for main-spec hints test/core/validation.test.ts	Adds a validateSpec test asserting the targeted "move the SHALL/MUST statement to the line immediately after the ... header" hint, and updates two delta-spec assertions to expect "requirement header" wording.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• Fission-AI/OpenSpec#1135: Earlier work on improving SHALL/MUST validation messaging via updates to validator.ts and buildMissingShallOrMustMessage.
• Fission-AI/OpenSpec#1154: Introduced clearer SHALL/MUST validation hints for change delta specs and related message-building logic.

Suggested reviewers

• TabishB
• alfred-openspec

Poem

🐰 In headers high the SHALL did peep,
But bodies silent, promises keep.
"Move it down," the rabbit cries with cheer,
So rules live where readers will hear.
Hops of joy — validation's clear!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(validator): hint main spec header-only SHALL/MUST' directly describes the main change—improving the validator hint for main specs when SHALL/MUST appears only in requirement headers.
Linked Issues check	✅ Passed	The PR directly addresses the linked issue #1156 by extending the existing header-only SHALL/MUST hint from change-delta specs to main specs, with new error conversion logic and comprehensive tests.
Out of Scope Changes check	✅ Passed	All changes are within scope: validator.ts modifications handle main spec Zod errors with tailored messages, helper methods are extended appropriately, and tests validate the new behavior without unrelated modifications.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/core/validation/validator.ts (1)

545-549: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Avoid hard-coding the ### Requirement: header shape in the fix hint.

Main specs in this suite are also accepted with bare ### ... requirement headers, so this remediation text can point users at a header form that does not exist in their file. Make the guidance refer to “the requirement header” generically instead.

💡 Suggested wording

   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} in the requirement body, not only in the header. Move the SHALL/MUST statement to the line immediately after the requirement header.`;
   }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/core/validation/validator.ts` around lines 545 - 549, The message in
buildMissingShallOrMustMessage hard-codes '### Requirement: ...' which may not
match actual headers; update the returned hint to reference "the requirement
header" generically instead of the literal header shape (this function name and
containsShallOrMust can be used to locate the logic). Modify the string returned
when containsShallOrMust(blockName) is true so it instructs the user to move the
SHALL/MUST statement to the line immediately after the requirement header (or
the requirement header line) rather than naming '### Requirement: ...'.

🧹 Nitpick comments (1)

test/core/validation.test.ts (1)

228-254: ⚡ Quick win

Add the same regression for the existing bare ### ... requirement style.

validateSpec already accepts main-spec requirements without the Requirement: prefix, but this new coverage only exercises the prefixed form. Adding one bare-header case here would protect the hint text for both supported syntaxes.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/core/validation.test.ts` around lines 228 - 254, Add a second sub-case
to this test that uses the bare main-spec requirement header style (e.g., "###
System SHALL only say the keyword here" without the "Requirement:" prefix) to
ensure Validator.validateSpec (via new Validator()) produces the same targeted
hint; replicate the same assertions (report.valid false, find issue at
'requirements.0.text' containing 'not only in the header' and 'Move the
SHALL/MUST statement', and assert the generic 'Requirement must contain SHALL or
MUST keyword' message is not present) so both syntaxes are covered.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/core/validation/validator.ts`:
- Around line 545-549: The message in buildMissingShallOrMustMessage hard-codes
'### Requirement: ...' which may not match actual headers; update the returned
hint to reference "the requirement header" generically instead of the literal
header shape (this function name and containsShallOrMust can be used to locate
the logic). Modify the string returned when containsShallOrMust(blockName) is
true so it instructs the user to move the SHALL/MUST statement to the line
immediately after the requirement header (or the requirement header line) rather
than naming '### Requirement: ...'.

---

Nitpick comments:
In `@test/core/validation.test.ts`:
- Around line 228-254: Add a second sub-case to this test that uses the bare
main-spec requirement header style (e.g., "### System SHALL only say the keyword
here" without the "Requirement:" prefix) to ensure Validator.validateSpec (via
new Validator()) produces the same targeted hint; replicate the same assertions
(report.valid false, find issue at 'requirements.0.text' containing 'not only in
the header' and 'Move the SHALL/MUST statement', and assert the generic
'Requirement must contain SHALL or MUST keyword' message is not present) so both
syntaxes are covered.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9e6a35e3-20e1-4ee0-b744-01583ca4ab85

📥 Commits

Reviewing files that changed from the base of the PR and between bc7ab26 and f72dc6d.

📒 Files selected for processing (2)

• src/core/validation/validator.ts
• test/core/validation.test.ts

[Pluviobyte]

Addressed the review feedback in 035622b:

• generalized the remediation hint to say “requirement header” rather than hard-coding ### Requirement: ...
• extended the main-spec regression to cover both prefixed and bare requirement headers
• updated the delta-spec assertions to match the generic wording

Validation run locally:

• npx vitest run test/core/validation.test.ts
• npx eslint src/core/validation/validator.ts test/core/validation.test.ts (0 errors; test file is ignored by the current ESLint config)