Skip to content

feat(ci): validate skill publishing rules on pull requests and on main - #125

Open
aadimch wants to merge 2 commits into
aws:mainfrom
aadimch:feat/validate-skill-publishing
Open

aadimch wants to merge 2 commits into
aws:mainfrom
aadimch:feat/validate-skill-publishing

Conversation

@aadimch

@aadimch aadimch commented Oct 7, 2026 •

Copy link
Copy Markdown

Description

This PR adds a validate-skill-publishing check, and moves two skills to full MAJOR.MINOR.PATCH versions so that they pass it.

Skills on main are published as one set, so one skill that breaks a rule stops every skill from being published. The check applies the publishing rules to every skill in skills/ on each pull request, so that a pull request that passes the check cannot put a skill on main that breaks a rule. The rules are in the new "Publishing Rules" section of CONTRIBUTING.md.

The check (.github/scripts/validate_skill_publishing.py):

  • Reads each skill with git archive at the head commit and at the merge base.
  • SKILL.md: valid UTF-8, a YAML frontmatter block, name (1 to 64 characters, ^[a-z0-9]+(-[a-z0-9]+)*$, equal to the folder name), description (1 to 1024 characters), metadata.version as a quoted MAJOR.MINOR.PATCH string, and metadata.deprecated as a boolean if present.
  • Plain YAML only: no duplicate keys, anchors, aliases, tags, directives, tabs in indentation, or other features that YAML parsers read in different ways.
  • Files: at most 100 published files, path rules, no symlinks, a zip of at most 983,040 bytes (1 MiB with a safety margin), and size limits for the repository archive.
  • Versions against the merge base: a published version is immutable. A change to a published file needs a higher metadata.version, the version never goes down, and a skill folder is never removed (deprecate it instead). evals/, .skilleval.yaml, and CHANGELOG.md are not published, so a change to them alone needs no bump. README.md is published, so a change to it needs a bump.
  • Content hash: SHA-256 over the sorted records <path>\0<file sha256 hex>\n of the published files, documented in the script and checked against a golden fixture.
  • Self-check: before it checks any skill, the script checks its own content hash against a golden fixture, the digest of .github/scripts/conformance/cases.json, and every one of the 53 conformance cases in that file. A failed self-check exits with code 2. Run it with --self-check-only.

Workflows:

  • validate-skill-publishing.yml runs on each pull request. It has read-only permissions, no paths filter (so that it always reports a status), passes event values through env, and does not keep the checkout credentials.
  • validate-skill-publishing-main.yml runs the check again after each push to main, so that two pull requests that each passed on an older base and break a rule together are reported at once.

Skill versions:

  • analytics-opensearch-expertise: 2.6 → 2.6.1
  • database-rds-devops: 1.0 → 1.0.1

Each gets a CHANGELOG.md entry. The patch number goes up, and not to .0, because the edit to SKILL.md changes the published content.

Requests for maintainers:

  1. Add validate-skill-publishing to the required status checks for main. Until then, the check is advisory.
  2. Turn on "Require branches to be up to date before merging", or use a merge queue. The check is exact only for the base commit that it ran on.
  3. Consider a CODEOWNERS entry for .github/scripts/ and .github/workflows/. A pull request that changes the check runs its own changed copy.

Effect on open work: with this check required, a change to a skill's README.md without a version bump fails. For example, #122 would have needed a patch bump for devops-agent-cost-insights.

Type of change

  • New skill
  • New custom agent
  • New MCP server
  • Update to an existing skill, agent, or MCP server
  • Documentation or infrastructure change

Testing

  • python3 .github/scripts/validate_skill_publishing.py --base-ref origin/main on this branch: the self-check passes, and all 30 skills pass (exit 0). The two changed skills report 2.6.0 → 2.6.1 and 1.0.0 → 1.0.1.
  • --self-check-only passes in 0.02 s. Each of these local changes makes the self-check fail with exit code 2: a changed hash record separator, an edited conformance case, and a conformance case with a flipped expected result.
  • Local scenarios in a scratch repository: a content change without a bump fails, a bump passes, a lower version fails, a change only to CHANGELOG.md or evals/ passes, a removed skill folder fails, a new skill passes, a duplicate key, a symlink, and a 513-character path fail.
  • --base-ref HEAD~30 on main reports the earlier removal of skills/eks-operation-review, as expected.
  • validate-skill-evals passes for the two changed skills, with the existing warnings for their legacy evals/ layout.
  • scan_aws_identifiers.py over the diff: 0 findings.

License confirmation

  • By submitting this pull request, I confirm that my contribution is made under the terms of the Apache License 2.0.

@aadimch
aadimch marked this pull request as ready for review October 7, 2026 01:25
@ams-thakkar
ams-thakkar self-requested a review October 8, 2026 13:05
@ams-thakkar

Copy link
Copy Markdown
Contributor

Holding this one, and #126–#129 likewise: @aadimch and @miscreantmoogly have each built the same check independently, and the two are not compatible as they stand — most pointedly, metadata.deprecated is specified oppositely (unquoted true here, quoted "true" in the stack), and each documents its own form in CONTRIBUTING.md.

I have asked the two of you to agree on which set to keep.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants