Skip to content

build(ci): add --base/--head changed-files support to validate-frontmatter - #2499

Open
ashleyshaw with Copilot wants to merge 14 commits into
developfrom
copilot/fix-missing-npm-script-validate-frontmatter-change
Open

build(ci): add --base/--head changed-files support to validate-frontmatter#2499
ashleyshaw with Copilot wants to merge 14 commits into
developfrom
copilot/fix-missing-npm-script-validate-frontmatter-change

Conversation

Copilot AI commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

The front-matter-validate CI check was failing on every PR because npm run validate:frontmatter:changed existed in package.json but the underlying script ignored the --base/--head SHA flags passed by the workflow — effectively running a full scan on every invocation and providing no "changed files only" behaviour.

Build/CI change

  • FileDiscovery.findChangedFiles(baseSha, headSha, ...) — new static method; runs git diff --name-only --diff-filter=ACMRT via execFileSync (arg array, no shell) and intersects results with the pattern-matched/excluded file set; falls back to full scan gracefully on any git error
  • SHA validation — rejects non-hex or out-of-range values before exec, eliminating the shell-injection vector CodeQL flagged
  • validateFrontmatter(filePaths?) — accepts an optional file list; CLI wires it up when both --base and --head are present, full scan otherwise
  • Promise rejection safety — both validateFrontmatter() call sites now chain .catch() so async rejections produce a non-zero exit rather than a silent pass
# CI invocation now resolves correctly
npm run validate:frontmatter:changed -- --base $BASE_SHA --head $HEAD_SHA
# → validates only .md/.yml files touched in the PR diff

Baseline & Target

  • Before: CI step always ran full repo scan (~500+ files); no SHA-scoped filtering despite the flag being passed
  • After: Only files changed between BASE_SHA and HEAD_SHA are validated; full fallback retained when git is unavailable

Rollback

Revert the single commit to scripts/validation/validate-frontmatter.js; the validate:frontmatter:changed npm script entry in package.json is unchanged.

Notes

  • Uses execFileSync (argument array) — no shell involved, no injection surface
  • SHA regex: /^[0-9a-f]{4,64}$/i — covers abbreviated and full SHAs

Changelog

Fixed


Checklist (Global DoD / PR)

  • All AC met and demonstrated
  • Tests added/updated (unit/E2E as appropriate)
  • Accessibility checklist completed (where relevant):
    • Semantic HTML and heading order verified
    • Keyboard navigation and visible focus states verified
    • ARIA used only where needed
    • Contrast and non-colour cues reviewed (WCAG 2.1 AA or higher)
  • Docs/readme/changelog updated (if user-facing)
  • Security checklist completed (where relevant):
    • Untrusted input validated and sanitised
    • Output escaped for its rendering context
    • Privileged actions enforce nonce and capability checks
    • No secrets/sensitive data introduced; OWASP risks reviewed
  • Code/design reviews approved
  • CI green; linked issues closed; release notes prepared (if shipping)

The CI workflow calls `npm run validate:frontmatter:changed -- --base $BASE_SHA
--head $HEAD_SHA` but the script previously ignored those flags and validated
all files. This change:

- Adds `FileDiscovery.findChangedFiles(baseSha, headSha, ...)` which uses
  `git diff --name-only --diff-filter=ACMRT` (via execFileSync, no shell
  interpolation) to get changed files, then intersects with the set of
  pattern-matched/non-excluded files; falls back to full scan on any error.
- Validates SHA format (4–64 hex chars) before use to prevent shell injection.
- Updates `validateFrontmatter(filePaths?)` to accept an optional file list.
- Parses `--base`/`--head` in the CLI and passes the changed-file list to the
  validator when both are present.
- Chains `.catch()` on both `validateFrontmatter()` call sites so unhandled
  rejections cause a non-zero exit (CI correctness fix).
- Updates `--help` output to document the new flags.

Co-authored-by: ashleyshaw <1805352+ashleyshaw@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix missing npm script 'validate:frontmatter:changed' in CI workflow build(ci): add --base/--head changed-files support to validate-frontmatter Aug 29, 2026
Copilot AI requested a review from ashleyshaw August 29, 2026 15:34
@ashleyshaw
ashleyshaw marked this pull request as ready for review August 29, 2026 16:33
@ashleyshaw
ashleyshaw requested a lite review from Copilot August 29, 2026 16:39
@ashleyshaw
ashleyshaw enabled auto-merge (squash) August 29, 2026 16:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

This PR fixes the validate:frontmatter:changed CI workflow path by adding --base/--head support so frontmatter validation can scope to only files changed between two git SHAs, while retaining a full-scan fallback.

Changes:

  • Added FileDiscovery.findChangedFiles(baseSha, headSha, ...) to compute changed files via git diff --name-only and filter them to supported patterns/exclusions.
  • Updated validateFrontmatter(filePaths?) and CLI arg parsing to validate only changed files when --base and --head are provided.
  • Ensured async validation invocations surface failures via .catch() with non-zero exit codes.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread scripts/validation/validate-frontmatter.js Outdated
Comment thread scripts/validation/validate-frontmatter.js Outdated
Comment thread scripts/validation/validate-frontmatter.js
Comment thread scripts/validation/validate-frontmatter.js Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Ash Shaw <ashley@lightspeedwp.agency>
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Reviewer Summary for PR #2499

CI Status:pending
Files changed: 1

Recommendations

  • CI checks not green

1 similar comment
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Reviewer Summary for PR #2499

CI Status:pending
Files changed: 1

Recommendations

  • CI checks not green

auto-merge was automatically disabled August 29, 2026 17:04

Head branch was pushed to by a user without write access

…e CI diagnostics

Co-authored-by: ashleyshaw <1805352+ashleyshaw@users.noreply.github.com>
@ashleyshaw

Copy link
Copy Markdown
Member

Merge readiness checklist for #2499 (develop)

Summary

  • Branch is up to date with develop (merged from develop on 29 August 2026)
  • PR change intent is correct: validate-frontmatter now supports --base/--head changed-files flow
  • Required checks are green
  • No unrelated file churn (confirm package-lock.json change is intentional)

1) Fix in this PR (must complete before merge)

  • Confirm/clean diff scope

    • Keep only intended CI/script changes
    • Revert package-lock.json if it is incidental and not required for this fix
  • Workflow checks (PR-related)

    • Re-run failing jobs after any scope cleanup
    • Fix only failures caused by this PR’s changes (scripts/validation/validate-frontmatter.js)
    • Re-run until required checks pass
  • Safety/quality gates

    • Script still falls back to full scan when git/SHA range is unavailable
    • Invalid SHA handling remains strict (--base/--head validation)
    • Promise rejection paths exit non-zero (no silent pass)

2) Track in separate issue (do not block this PR if unrelated)

For each failing CI error that is not caused by this PR:

Unrelated CI issues opened:

  • # —
  • # —
  • # —

3) Final approval + merge steps

  • At least one maintainer approval
  • Required status checks green
  • No unresolved blocking conversations
  • Merge method selected (Squash recommended for single logical change)
  • Merge into develop

Suggested squash commit message:
build(ci): scope frontmatter validation to changed files via --base/--head


Post-merge quick verification (develop)

  • Confirm front-matter-validate runs with changed-files behaviour
  • Confirm no regression in full-scan fallback path
  • Confirm linked unrelated CI issues are triaged/assigned

@github-actions

Copy link
Copy Markdown
Contributor

🔍 Reviewer Summary for PR #2499

CI Status:pending
Files changed: 2

Recommendations

  • CI checks not green

1 similar comment
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Reviewer Summary for PR #2499

CI Status:pending
Files changed: 2

Recommendations

  • CI checks not green

@github-actions

github-actions Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

🚫 This PR description is missing required template content.

Missing required section(s): Linked issues, Global DoD checklist

Please update the PR body using one of the repository PR templates:

Empty placeholders, unchecked checklist boxes, and stub issue references do not count.

@ashleyshaw ashleyshaw added this to the v1.1 milestone Aug 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Branch Naming Issue

This PR's branch name copilot/fix-missing-npm-script-validate-frontmatter-change doesn't follow the standard pattern.

Standard pattern: {type}/{scope}-{title} (e.g., feat/something, fix/bug-name)

Based on linked issue type: bug

Correct template should be: pr_bug.md

No action required — this is informational. Future PRs should use the correct branch name.

See Branching Strategy for more info.

@github-actions

Copy link
Copy Markdown
Contributor

🔍 Reviewer Summary for PR #2499

CI Status:pending
Files changed: 2

Recommendations

  • CI checks not green

1 similar comment
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Reviewer Summary for PR #2499

CI Status:pending
Files changed: 2

Recommendations

  • CI checks not green

Copilot AI and others added 5 commits September 2, 2026 00:55
The CI workflow calls `npm run validate:frontmatter:changed -- --base $BASE_SHA
--head $HEAD_SHA` but the script previously ignored those flags and validated
all files. This change:

- Adds `FileDiscovery.findChangedFiles(baseSha, headSha, ...)` which uses
  `git diff --name-only --diff-filter=ACMRT` (via execFileSync, no shell
  interpolation) to get changed files, then intersects with the set of
  pattern-matched/non-excluded files; falls back to full scan on any error.
- Validates SHA format (4–64 hex chars) before use to prevent shell injection.
- Updates `validateFrontmatter(filePaths?)` to accept an optional file list.
- Parses `--base`/`--head` in the CLI and passes the changed-file list to the
  validator when both are present.
- Chains `.catch()` on both `validateFrontmatter()` call sites so unhandled
  rejections cause a non-zero exit (CI correctness fix).
- Updates `--help` output to document the new flags.

Co-authored-by: ashleyshaw <1805352+ashleyshaw@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Ash Shaw <ashley@lightspeedwp.agency>
…e CI diagnostics

Co-authored-by: ashleyshaw <1805352+ashleyshaw@users.noreply.github.com>
@ashleyshaw
ashleyshaw force-pushed the copilot/fix-missing-npm-script-validate-frontmatter-change branch from caeb4cf to 6ed087d Compare September 1, 2026 17:56
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

❌ Branch Name Validation Failed

The branch name copilot/fix-missing-npm-script-validate-frontmatter-change does not follow the LightSpeed branching strategy.

Required Format

{type}/{scope}-{short-title}
  • type: one of the allowed prefixes (lowercase)
  • scope: lowercase, hyphens only (no underscores or uppercase)
  • title: lowercase, hyphens only (no underscores or uppercase)

Allowed Branch Types

feat, fix, hotfix, release, refactor, chore, docs, test, perf, ci, build, deps, security, revert, research, design, a11y, ux, i18n, ops, proto, ds, api, schema, telemetry, content, seo, config, migrate, qa, uat, audit, codex

Valid Examples

  • feat/branch-naming-enforcement
  • fix/validation-script-bug
  • chore/update-dependencies
  • docs/branching-strategy-guide
  • hotfix/critical-security-patch

Invalid Examples

  • claude/my-branch (type "claude" not allowed)
  • Feature/MyBranch (uppercase not allowed)
  • fix-bug (missing type prefix)
  • feat/my_feature (underscores not allowed)
  • feat/MyFeature (uppercase not allowed)

Solution

Rename your branch to follow the pattern and update the PR.

For more information, see docs/BRANCHING_STRATEGY.md.

Co-authored-by: ashleyshaw <1805352+ashleyshaw@users.noreply.github.com>
auto-merge was automatically disabled September 2, 2026 17:03

Head branch was pushed to by a user without write access

@ashleyshaw
ashleyshaw enabled auto-merge September 2, 2026 18:44
@claude claude Bot added area:ci Build and CI pipelines area:scripts Scripts & tooling area:tests Test suites & harnesses lang:js JavaScript/TypeScript lang:json JSON config/content type:bug Bug or defect labels Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:ci Build and CI pipelines area:scripts Scripts & tooling area:tests Test suites & harnesses lang:js JavaScript/TypeScript lang:json JSON config/content type:bug Bug or defect

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: Missing npm script 'validate:frontmatter:changed' in CI workflow

3 participants