Review Pr

package-spec PR Review (Agent Skill)

This skill conforms to the Agent Skills open format (agentskills.io).

This skill does not modify the codebase. It does not edit source files, run any git command that modifies state (no commit, push, checkout, reset, stash, merge, or rebase), or change anything on the branch under review. If an issue is found, describe it and stop — do not attempt to fix it. The only file it writes is the review report at .cursor/review-pr-output.md.

Reviews a branch or pull request against the contribution rules for this repository. See references/checklist.md for full rules and examples.

For narrative onboarding (Makefile targets, scaffolding test packages, testing against the integrations repo), see CONTRIBUTING.md at the repository root. This skill and the checklist focus on PR completeness against those practices.

How to Use This Skill

Step 1 — Resolve the Target

Determine what to diff based on the input provided:

If a PR number was given (e.g., 1140):

gh pr view 1140 --repo elastic/package-spec --json headRefName,baseRefName,title,url
git fetch origin <headRefName>
# diff: FETCH_HEAD vs <baseRefName>
git diff <baseRefName>...FETCH_HEAD --stat
git diff <baseRefName>...FETCH_HEAD

If a branch name was given (e.g., my-feature):

git diff main...<branch> --stat
git diff main...<branch>

If no argument was given (review current branch):

git branch --show-current          # confirm we are not on main
git diff main...HEAD --stat
git diff main...HEAD

If the current branch is main and no argument was given, report that there is nothing to review.

Step 2 — Categorize Changed Files

Using the file list from --stat, bucket each changed file into one or more categories:

Use the categories internally to pick checklist sections; do not paste the full categorized file list into the report unless it clarifies an extra-review item.

Step 3 — Run the Checklist

Work through each applicable section from references/checklist.md.

Below is a compact version of the checks. The reference file has full rules, edge cases, and exact examples.

Contribution process (when relevant)

Consult section 0 in the reference file and CONTRIBUTING.md.

• Cross-stack or cross-product spec changes should follow the Change Proposals process before merge.
• New package categories require the Category Proposals process.
• If the change is large and the PR does not reference a proposal or tracking issue, add an extra-review note asking the author to confirm process was followed.

Always: Changelog

• Is spec/changelog.yml modified?
  • If not: flag as missing (required for all non-trivial changes; dependency-bump PRs from Dependabot are the only exception).
• If modified, verify:
  • Entry is under the correct in-development version (the one with -next suffix).
  • New entry is at the bottom of that version's changes list.
  • description is a complete sentence ending with a period.
  • type is one of: enhancement, bugfix, or breaking-change.
  • link points to a valid PR URL or is TBD (not empty, not an issue URL unless the change originated from an issue with no PR).
  • If the feature is blocked on Kibana/elastic-package: a # Pending on <url> comment appears directly above the entry.

If spec files changed

Consult the "Version patches" and "Schema reuse" sections of the reference file.

• For new properties or definitions that older format_version values must not include, add versions[].patch remove operations (see the reference file). Not every addition needs a patch: if the change is only meaningful at the introducing spec version (e.g. a new required field or other breaking change with a clear minimum format_version), rely on version scoping instead of remove patches when appropriate.
• The remove order matters: property $ref entries must be removed before the definition they reference.
• Shared fields used in multiple spec files must be defined once in spec/integration/manifest.spec.yml and referenced via $ref elsewhere — not duplicated inline.

If semantic validators changed

Consult the "Semantic validators" section of the reference file.

• New validator function: check it is registered in code/go/internal/validator/spec.go in the rulesDef slice, with appropriate fn, since, and types fields.
• Tests: at least one of the following must be present:
  • A *_test.go file alongside the validator using t.TempDir() (simple/single-file rules).
  • New test cases in code/go/pkg/validator/validator_test.go referencing packages in test/packages/ (complex multi-file scenarios).

If test packages changed

Consult the "Test packages" section of the reference file.

• Prefer scaffolding with elastic-package create package or, for new data streams in an existing package, elastic-package create data-stream (from that package’s directory), over manual copies.
• Each package under test/packages/ or compliance/testdata/packages/ must contain: manifest.yml, changelog.yml, docs/README.md.
• Any data stream must include a valid ingest pipeline: processors need a tag field, and there must be an on_failure block that sets event.kind: pipeline_error and error.message to a string containing the three Handlebars placeholders.

If compliance feature files changed

Consult the "Compliance features" section of the reference file.

• Every Scenario: must have a @X.Y.Z version tag immediately above it.
• A @skip tag must always be paired with a # Pending on <url> comment on the next line.
• When @skip is added, the corresponding spec/changelog.yml entry must also have a matching # Pending on <url> comment.

If new Go files added

• Every new .go file must start with the Elastic license header:

  // Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
  // or more contributor license agreements. Licensed under the Elastic License;
  // you may not use this file except in compliance with the Elastic License.
  

• Variable and function names must be unabbreviated (e.g., packageName not pkgName, dataStreamManifests not dsManifests).

• Indentation must use tabs, not spaces.

Step 4 — Run Local Validation

If a Go toolchain and make are available in the environment, run at minimum (same order as CONTRIBUTING.md):

# Validate spec YAML syntax (fast, always do this after spec changes)
go test ./code/go/internal

# Lint and license headers
make check

Optional (broader validation): When the diff warrants it, also run make test for the full suite (CONTRIBUTING.md). For changes that should be exercised across many real packages, see Testing with integrations repository in CONTRIBUTING.md (e.g. test integrations PR comment and closing the spawned integrations PR when done).

Record validation outcomes only as described in Step 5 (short summary per command; on failure, include a small excerpt only). If the toolchain is not available, note skipped for those commands.

Step 5 — Write the Report

Write a short report to .cursor/review-pr-output.md, overwriting any previous run. Use plain text only (no emoji or decorative symbols). Aim for roughly one screen of content.

Include only what follows. Omit empty sections entirely.

# PR review: <branch or PR title>
<!-- reviewed: <ISO date> -->

## Scope

At most 2–4 bullets: what the diff changes, only if that context helps a reviewer. Omit this section if the PR title and `git diff --stat` are enough and nothing needs extra review.

## Extra review

Bullets **only** for items that need human attention: checklist gaps, blockers, open questions, or parts of the change that are easy to get wrong (for example new `versions` patches, new semantic validators, `spec/changelog.yml` entries, compliance `@skip`, or cross-file `$ref`). For each bullet use `path:approximate-line` (or `path` and a short region description) plus **one sentence**. Prefix with `blocker`, `suggestion`, or `question` when useful.

If there is nothing to flag here and validation passed, use a single line instead of a list:

No items flagged for extra review.

## Validation

One line per command you ran, for example: `go test ./code/go/internal: pass`. Use `fail` or `skipped` as appropriate. If a command failed, add a **short** excerpt (failing test name or error lines only), not the full log. If no commands ran, one line: `Validation: skipped (toolchain not available).`

Do not add per-topic sections (changelog, version patches, tests, etc.) when there is nothing to say. Do not paste full go test or make check output on success. Do not list files that look fine.

After writing the file, print this message to the user:

Review written to .cursor/review-pr-output.md

Validating this skill

After substantive edits to this skill, run (if installed):

skills-ref validate ./.cursor/skills/review-pr