microsoft / microsoft/hve-core

fix(linting): frontmatter schema overlay never enforces and misroutes 231 artifact files

Open
#1,553 2 comments 0 reactions 2 assignees Claimed by @jkim323 View on GitHub
agent-ready bug linting maintenance scripts tech-debt
Dominant language
Python
Stars
1.5k
Forks
301
Avg merge
3d 3h
Merged PRs (30d)
92

Description

## Context

This issue previously described consolidating ADR consistency enforcement into the frontmatter validator. That description did not match the repository, so the issue has been re-scoped to the two real defects found while investigating it. See the "What changed and why" section at the bottom for the correction record.

`scripts/linting/Validate-MarkdownFrontmatter.ps1` carries a JSON Schema overlay that routes each markdown file to a schema from `scripts/linting/schemas/schema-mapping.json` and validates its frontmatter. `npm run lint:frontmatter` invokes it with `-WarningsAsErrors -EnableSchemaValidation`, so the overlay is active on every local and CI run.

Two defects mean that overlay currently validates almost nothing, and could not fail a build even if it did.

## Defect 1: `**` in schema-mapping patterns matches at most one path segment

`Validate-MarkdownFrontmatter.ps1:229-234` converts a glob to a regex in three ordered replacements:

```powershell
$regexPattern = $rule.pattern -replace '\.', '\.'
$regexPattern = $regexPattern -replace '\*\*/', '(.*/)?'
$regexPattern = $regexPattern -replace '\*', '[^/]*'
```

The third replacement rewrites the `*` **inside the `(.*/)?` that the second replacement just inserted**, producing `(.[^/]*/)?`. That group matches a single path segment instead of any number.

For the mapping `.github/**/*.agent.md` the emitted regex is:

```text
^\.github/(.[^/]*/)?[^/]*\.agent\.md$
```

| Path | Matches |
|---------------------------------------------------------|---------|
| `.github/agents/x.agent.md` | yes |
| `.github/agents/project-planning/adr-creation.agent.md` | no |

Anything nested more than one level below the glob prefix falls through to `defaultSchema` (`base-frontmatter.schema.json`).

### Measured impact

Across `docs/` and `.github/` (1128 files, excluding `node_modules` and `docusaurus/build`), 1005 resolve to `base-frontmatter.schema.json`. Of the 241 artifact files that have a dedicated schema, **231 are misrouted**:

| Artifact type | Misrouted | Intended schema |
|---------------------|-----------|---------------------------------------|
| `*.agent.md` | 55 | `agent-frontmatter.schema.json` |
| `*.instructions.md` | 56 | `instruction-frontmatter.schema.json` |
| `*.prompt.md` | 48 | `prompt-frontmatter.schema.json` |
| `SKILL.md` | 72 | `skill-frontmatter.schema.json` |

**No `SKILL.md` file in the repository has ever been validated against `skill-frontmatter.schema.json`.** All 72 fall through to the generic base schema.

`docs/**/*.md`, the ADR pattern, and the pipe-separated root-community pattern currently resolve correctly, so any fix must preserve their behavior.

## Defect 2: schema violations are detected and then discarded

`Validate-MarkdownFrontmatter.ps1:795-808`:

```powershell
# Optional schema validation overlay (advisory only)
if ($EnableSchemaValidation -and (Initialize-JsonSchemaValidation)) {
foreach ($fileResult in $summary.Results) {
...
if ($schemaResult.Errors.Count -gt 0) {
Write-Warning "JSON Schema validation errors in $($fileResult.FilePath)"
$schemaResult.Errors | ForEach-Object { Write-Warning " - $_" }
}
...
}
}
```

`$schemaResult.Errors` is never added to `$summary`, so a schema violation cannot influence the exit code. `-WarningsAsErrors` does not reach it either, because these are `Write-Warning` calls rather than validation findings.

The overlay itself is a real implementation, not a stub. `Test-JsonSchemaValidation` covers `required`, `type`, `properties`, `items`, `oneOf`, `pattern`, `enum`, and `minLength`. Given a deliberately invalid ADR payload it returns six correct errors spanning four keyword families. Its documented non-goals are `$ref`, `allOf`/`anyOf`, and `additionalProperties`.

## Why the two defects mask each other

A full-tree run today reports **898 files, 0 errors, 0 warnings, exit 0** with zero schema-error lines. That looks like a clean corpus. It is substantially a consequence of Defect 1: the schemas that would catch real problems are barely reachable, and anything they did catch would be discarded by Defect 2.

## Measured remediation cost

Simulating corrected routing across all 241 artifact files against their correct schemas produces **exactly one violation**:

```text
.github/skills/rai/rai-standards/SKILL.md
Field 'metadata.last_updated' does not match required pattern: ^\d{4}-\d{2}-\d{2}$
```

The file has `last_updated: "2024"` where the schema requires `YYYY-MM-DD`. This is a genuine data defect that is invisible today.

So both defects can be fixed and enforcement enabled for a one-field correction. There is no large latent backlog.

## Scope

- [ ] Fix the glob-to-regex translation at `scripts/linting/Validate-MarkdownFrontmatter.ps1:229-234` so `**` matches any number of path segments. Avoid the self-rewrite by escaping the pattern first, or by ordering replacements so the `**` substitution is not reprocessed.
- [ ] Correct `metadata.last_updated` in `.github/skills/rai/rai-standards/SKILL.md` to an ISO 8601 `YYYY-MM-DD` value.
- [ ] Promote the overlay from advisory to enforcing: feed `$schemaResult.Errors` into the same `ValidationSummary` exit-code path used by frontmatter findings.
- [ ] Add Pester coverage for glob resolution: one-level match, multi-level match, non-matching path, and the currently-correct `docs/**/*.md` and pipe-separated root-community patterns.
- [ ] Add Pester coverage asserting that a schema violation produces a non-zero exit from the frontmatter validator.

### Sequencing

Steps 1 and 2 must land before or with step 3. Enabling enforcement first would gate the build on the generic base schema while still missing every artifact-specific rule.

## Out of scope

- `scripts/linting/Validate-AdrConsistency.ps1`, `npm run lint:adr-consistency`, and `.github/workflows/adr-consistency-validation.yml` stay exactly as they are. That workflow runs with `soft-fail: false` and is a member of the required-check aggregate in `pr-validation.yml`. It is a separate concern: 7 of its 9 rules are `body` or `cross-region` scope and depend on `Modules/AdrBodyParser.psm1`, so they cannot be expressed as frontmatter schema checks.
- Adding `$ref`, `allOf`, `anyOf`, or `additionalProperties` support to `Test-JsonSchemaValidation`.
- Changing any schema's contents or the mappings in `schema-mapping.json`.

## Acceptance criteria

- `.github/**/*.agent.md`, `.github/**/*.instructions.md`, `.github/**/*.prompt.md`, and `.github/skills/**/SKILL.md` resolve to their dedicated schemas at any nesting depth.
- `docs/**/*.md`, `docs/planning/adrs/NNNN-*.md`, and the root-community pipe pattern continue to resolve as they do today.
- A frontmatter schema violation causes `npm run lint:frontmatter` to exit non-zero.
- `npm run lint:frontmatter` passes on a clean tree after the `last_updated` correction.
- `npm run test:ps` passes, including the new glob-resolution and enforcing-overlay cases.
- `npm run lint:adr-consistency` and the ADR consistency workflow are unchanged and still pass.

## What changed and why

The original issue stated that an ADR overlay existed at `Validate-MarkdownFrontmatter.ps1:807` calling `Invoke-AdrConsistencyValidation`, and proposed retiring the standalone ADR validator, its npm target, and a "previously-considered" workflow. Verification against `main` found:

- `git log -S 'Invoke-AdrConsistencyValidation' -- scripts/linting/Validate-MarkdownFrontmatter.ps1` returns nothing. That call has never existed in the file. Line 807 is the closing brace of the generic JSON Schema block described above. PR #1552 changed this file by exactly one line, an unrelated `ExcludePaths` entry.
- `.github/workflows/adr-consistency-validation.yml` exists, runs with `soft-fail: false`, uploads SARIF, and is listed in the `pr-validation.yml` required-check aggregate. Cancelling it would remove an enforcing gate.
- ADR consistency is not a frontmatter concern: 5 of 9 rules are `body` scope and 2 are `cross-region`, backed by a dedicated body parser.
- `docs/planning/adrs/0001-*.md` lists `scripts/linting/Validate-AdrConsistency.ps1` in its own `affected_components`, which two of those rules constrain against the body. Deleting the script would force edits to an accepted ADR whose Decision Outcome cites it.

Following the original checklist would therefore have reduced CI coverage rather than consolidating it. The surviving intent, "promote from advisory to enforcing", is preserved above and applied to the overlay that actually exists.

The `last_updated` finding is adjacent to #2648 (`metadata.last_updated` has no freshness validator). Fixing the format here is a prerequisite for any freshness threshold there.

🤖 Crafted with precision by ✨Copilot following brilliant human instruction, then carefully refined by our team of discerning human reviewers.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.