Skip to content

Add pack validation to generate.py and CI#11

Open
breferrari wants to merge 3 commits into
mainfrom
feat/pack-validation
Open

Add pack validation to generate.py and CI#11
breferrari wants to merge 3 commits into
mainfrom
feat/pack-validation

Conversation

@breferrari

Copy link
Copy Markdown
Contributor

Summary

  • Add validate_pack() and validate_unique_server_names() to catch malformed packs before publishing
  • Validates: required fields, name format, semver, server names/transport/command, cross-pack uniqueness
  • Add --validate-only flag for local pre-commit use
  • Add validate.yml workflow that runs on PRs touching src/** or scripts/generate.py
  • Includes tomllib change from Replace regex TOML parsing with stdlib tomllib #10 as prerequisite

Test plan

  • All 13 existing packs pass validation
  • --validate-only exits without writing files
  • Full generation produces byte-for-byte identical output
  • CI passes on this PR

Closes #6

🤖 Generated with Claude Code

breferrari and others added 2 commits March 25, 2026 09:44
The regex-based parse_pack_toml() worked for simple flat fields but would
break on multiline strings, inline tables, or other valid TOML constructs.
Switch to Python's stdlib tomllib (3.11+) for spec-compliant parsing.

Pin Python 3.12 in CI via actions/setup-python to guarantee tomllib
availability regardless of runner image updates.

Output is byte-for-byte identical to the regex-based parser for all 13
current packs.

Closes #5

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add validate_pack() and validate_unique_server_names() that check:
- Required fields (name, version, description)
- Name format (lowercase, digits, hyphens, no trailing hyphen)
- Semver format (X.Y.Z)
- authors/keywords are string lists
- [[servers]] syntax (not [servers])
- Server names: format, uniqueness within and across packs
- Server command/url required based on transport type

Validation runs before generation on every invocation. Add
--validate-only flag for local/CI use without writing files.

Add validate.yml workflow that runs on PRs touching src/** or
scripts/generate.py so contributors get feedback before merge.

Closes #6

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 25, 2026 08:47
@breferrari breferrari mentioned this pull request Mar 25, 2026
3 tasks

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a validation phase to scripts/generate.py (and new CI workflow) so malformed packs are caught early, before registry JSON generation/publishing.

Changes:

  • Add validate_pack() and validate_unique_server_names() and run them before generation, with a --validate-only mode for local/CI use.
  • Switch TOML parsing to tomllib (and pin CI Python to 3.12) to support full-structure validation.
  • Add a validate.yml workflow to run validation on PRs that touch src/** or scripts/generate.py.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
scripts/generate.py Adds TOML parsing via tomllib, new validation helpers, and a --validate-only flag; runs validation before generating registry files.
.github/workflows/validate.yml New PR workflow that runs generate.py --validate-only on relevant changes.
.github/workflows/publish.yml Pins Python 3.12 so generate.py can rely on tomllib during publish regeneration.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread scripts/generate.py Outdated
except (ValueError, tomllib.TOMLDecodeError):
continue

for server in data.get("servers", []):

Copilot AI Mar 25, 2026

Copy link

Choose a reason for hiding this comment

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

validate_unique_server_names() assumes data["servers"] is a list of tables and that each element is a dict. If a pack incorrectly uses [servers] (dict) or servers = ["..."], this function will iterate wrong types and can crash with AttributeError, masking the earlier validation errors. Mirror the type checks from validate_pack() (ensure list, then ensure each server is a dict) before accessing .get().

Suggested change
for server in data.get("servers", []):
servers = data.get("servers")
if servers is None:
continue
if not isinstance(servers, list):
# Malformed servers section; primary validation will report this.
continue
for server in servers:
if not isinstance(server, dict):
# Skip invalid server entries to avoid masking earlier errors.
continue

Copilot uses AI. Check for mistakes.
Comment thread scripts/generate.py
return errors

seen_server_names: set[str] = set()
for i, server in enumerate(servers):

Copilot AI Mar 25, 2026

Copy link

Choose a reason for hiding this comment

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

servers is validated to be a list, but individual entries aren’t type-checked. If a manifest uses servers = ["foo"] (or otherwise produces a non-dict entry), server.get(...) will raise an AttributeError and crash validation instead of returning a clear error. Add an isinstance(server, dict) check inside the loop and report an error/continue when it’s not a table.

Suggested change
for i, server in enumerate(servers):
for i, server in enumerate(servers):
if not isinstance(server, dict):
errors.append(
f"{pack_dir_name}: server #{i + 1} must be a table/object "
f"(got {type(server).__name__})"
)
continue

Copilot uses AI. Check for mistakes.
- Add isinstance check for [pack] section (must be a table, not scalar)
- Add string type checks for required fields (name, version, description)
- Guard server entries with isinstance(server, dict) checks
- Harden validate_unique_server_names() against non-list/non-dict servers
- Skip cross-pack validation when per-pack errors already exist
- Use consistent `python` (not `python3`) in CI workflows
- Add scripts/generate.py to publish.yml path triggers

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.

Add pack validation to generate.py and CI

2 participants