Proposal: automated PR validation checks for common XDM contribution mistakes

[ejsuncy]

Context

Companion issue to #2147 (CLAUDE.md). CLAUDE.md documents reviewer-enforced conventions so contributors (human and AI-assisted) can self-correct before opening a PR. This issue proposes programmatic enforcement so contributors don't have to read a doc to get the PR right — CI blocks the PR or the bot comments the fix directly.

CircleCI today runs npm run lint, mocha test, npm run xed-validation, and npm run incompatibility-check. It does not run npm run validate in CI, and most of the repeat reviewer feedback patterns below are not enforced anywhere.

Proposed checks

Ordered roughly by value-to-effort ratio. Each item is independently shippable.

1. Run npm run validate in CI

bin/validate-schemas.js catches example files that reference fields not defined in the schema, enum values not in the enum array, type mismatches, etc. Running it in CI (gated on "zero new failures vs master baseline" — the master baseline has ~58 pre-existing unrelated failures) would catch the single most common PR regression.

Effort: add - run: npm run validate to .circleci/config.yml. The script already exits non-zero on failure. Baseline-diff logic can be added as a follow-up.

2. meta:enum coverage check

Lint rule: for every schema under components/ / schemas/ / extensions/, every value listed in an enum array must appear as a key in the sibling meta:enum object. Missing keys fail CI with a file-and-path pointer.

Covers the #1 reviewer feedback ("forgot the meta:enum key").

Effort: small standalone Node script, callable from npm scripts. ~50 LOC with AJV's schema walker.

3. Example-schema field-drift check

Stricter version of npm test: for each *.example.*.json, assert every xdm:-prefixed property path resolves to a definition somewhere in the merged schema (including inherited allOf chains). Today this is caught implicitly when the schema uses additionalProperties: false; many XDM schemas don't set that, so invented fields in examples pass mocha.

Effort: medium. Reuse bin/validate-schemas.js plumbing; add strict-mode wrapper.

4. meta:status presence check

Every new schema (added in the PR diff) must set meta:status. Today this is reviewer-enforced; a git diff --name-only upstream/master...HEAD | grep '.schema.json$' | xargs jq ... one-liner in CI would flag omissions.

Effort: small. Pure shell + jq in CI.

5. PR body / metadata checks (GitHub Action)

An Action that, on PR open / synchronize:

• Requires Closes #<n> or Fixes #<n> in the body
• Requires the needs review label before a reviewer is auto-requested
• Detects rename/remove of existing properties in a schema diff and posts a "consider deprecating instead" comment with a link to the deprecation pattern in CLAUDE.md
• Detects conversion of a plain string property to an enum-constrained property and posts a "consider using meta:enum on a soft enum instead" comment

Effort: medium, one reusable workflow.

6. Auto-fix bot (stretch)

For the subset of issues that have a deterministic fix (missing meta:enum keys for values that exist in enum, xdm: prefix missing on a property that the schema defines with the prefix, prettier reformatting), a GitHub Action bot that opens a fix-commit PR branch or leaves suggestions.

Effort: large; only worth doing after 1–5 are in place and we have usage data on the most common violations.

7. Reuse-over-reinvent hint (stretch)

Harder to automate, but a suggest-existing-datatypes.js helper that takes a new datatype file and surfaces the top-N semantically similar existing datatypes (by title + property-name Jaccard similarity) would save design-review round-trips. Could run as a PR comment, not a blocking check.

Effort: medium–large. Likely worth only after 1–5.

Rollout

• Ship 1–4 behind a CI job that runs but does not block, for a 2-week observation window. Count true-positives vs false-positives.
• Promote to blocking once the false-positive rate is known and the baseline-diff UX is sound.
• 5–7 follow later as separate PRs.

Non-goals

• Changing any existing contributor-facing workflows beyond adding checks.
• Replacing the XDM team's human review — these checks catch mechanical mistakes so reviewers can spend their time on design.

Happy to take any or all of these on as follow-up PRs once there's signal on which ones are most valuable to the team.

[ejsuncy]

Addition: opt-in /ready slash command + onboarding bot comment

Context: #2149 review surfaced that the needs review label currently requires admin rights to apply, so the guidance in README was not actionable for external contributors. Rather than put the label behind admin-only access (or require contributors to remember to open PRs as draft), an opt-in slash-command workflow works for the fork-PR case.

Behavior

1. PR opens (any state). A GitHub Action posts a single onboarding comment from github-actions[bot]:

   👋 Thanks for the PR. When the change is ready for review, please comment /ready on this PR. If you need to mark it not-ready later, comment /wip to remove the label. Reviewers filter the open-PR list by the needs review label.

2. Contributor comments /ready (as its own line in any comment). Action:

   • Adds the needs review label.
   • Reacts 👍 on the comment so the contributor sees it registered.

3. Contributor comments /wip. Action removes the label, reacts 👀.

4. Optional: on pull_request.synchronize (new push after /ready), no action — label persists until /wip or merge.

Why opt-in over auto-label

• External contributors lack label permissions on adobe/xdm. Slash-command handler runs in base-repo context with GITHUB_TOKEN, which does have pull-requests: write.
• Not every PR author knows the convention exists. The onboarding comment teaches it at the moment it matters (PR open), rather than requiring contributors to read README or CLAUDE.md beforehand.
• Reviewers retain the option to look beyond the label at unlabelled open PRs — the label is a reviewer hint, not a gate.

Minimal workflow

name: PR review-ready slash commands
on:
  pull_request:
    types: [opened]
  issue_comment:
    types: [created]
permissions:
  pull-requests: write
  issues: write
jobs:
  onboard:
    if: github.event_name == 'pull_request'
    runs-on: ubuntu-latest
    steps:
      - uses: actions/github-script@v7
        with:
          script: |
            await github.rest.issues.createComment({
              owner: context.repo.owner, repo: context.repo.repo,
              issue_number: context.issue.number,
              body: "👋 Thanks for the PR. When the change is ready for review, " +
                    "please comment `/ready` on this PR. If you need to mark it " +
                    "not-ready later, comment `/wip` to remove the label. " +
                    "Reviewers filter the open-PR list by the `needs review` label."
            });
  ready-wip:
    if: github.event_name == 'issue_comment' && github.event.issue.pull_request
    runs-on: ubuntu-latest
    steps:
      - uses: actions/github-script@v7
        with:
          script: |
            const body = context.payload.comment.body;
            const isReady = /^\s*\/ready\b/m.test(body);
            const isWip   = /^\s*\/wip\b/m.test(body);
            if (!isReady && !isWip) return;
            const args = {
              owner: context.repo.owner, repo: context.repo.repo,
              issue_number: context.issue.number
            };
            if (isReady) {
              await github.rest.issues.addLabels({ ...args, labels: ['needs review'] });
              await github.rest.reactions.createForIssueComment({
                ...args, comment_id: context.payload.comment.id, content: '+1'
              });
            } else {
              await github.rest.issues.removeLabel({ ...args, name: 'needs review' }).catch(e => { if (e.status !== 404) throw e; });
              await github.rest.reactions.createForIssueComment({
                ...args, comment_id: context.payload.comment.id, content: 'eyes'
              });
            }

Rollout notes

• Onboarding comment idempotency: if the Action is added after this repo already has open PRs, limit the opened-type trigger to future PRs only; don't back-fill.
• Spam prevention: only react/label on comments where /ready or /wip is on its own line so random prose doesn't trigger it.
• Could also accept /needs-review as an alias for /ready for clarity.

Happy to take this as its own PR if the approach looks good.