Add pack deprecation and removal workflow#13
Conversation
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>
Add deprecation support:
- Read deprecated/deprecated_message from [pack] in pack.toml
- Propagate to packs/{name}.json and index.json
- Print [DEPRECATED] warnings during generation
Add removal support:
- Remove orphaned packs/*.json when src/ directory is deleted
- Ensures removed packs disappear from index.json
Document both workflows in CONTRIBUTING.md.
New fields are safe for Weave — PackMetadata and PackListing structs
use serde defaults and silently ignore unknown fields.
Closes #8
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds first-class support for pack deprecation/removal in the registry generator, plus CI/docs to standardize these workflows across contributions.
Changes:
- Parse
pack.tomlwithtomllib, validating pack metadata and server definitions; add--validate-onlymode. - Propagate
deprecated/deprecated_messageinto generatedpacks/*.jsonand deprecation flags intoindex.json, and remove orphanedpacks/*.jsonwhen a pack is deleted fromsrc/. - Document deprecation/removal steps in
CONTRIBUTING.mdand add a PR validation workflow; ensure publish workflow provisions Python 3.12.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| scripts/generate.py | TOML parsing + validation, deprecation propagation, orphan cleanup, --validate-only CLI |
| CONTRIBUTING.md | Documents deprecation and removal workflows |
| .github/workflows/validate.yml | Adds PR-time validation job using --validate-only |
| .github/workflows/publish.yml | Ensures Python 3.12 is installed before generation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return errors | ||
|
|
||
| seen_server_names: set[str] = set() | ||
| for i, server in enumerate(servers): |
There was a problem hiding this comment.
validate_pack() assumes each entry in servers is a dict and calls server.get(...). If a pack.toml has servers = [...] (array of scalars) or otherwise contains a non-table element, validation will raise an AttributeError and crash instead of reporting a useful error. Add an isinstance(server, dict) check (and emit an error like "server #N must be a table") before accessing .get().
| 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") | |
| continue |
| except (ValueError, tomllib.TOMLDecodeError): | ||
| continue | ||
|
|
||
| for server in data.get("servers", []): |
There was a problem hiding this comment.
validate_unique_server_names() iterates for server in data.get("servers", []) and then calls server.get(...). If servers is not a list (e.g. [servers] table) or contains non-table elements, this will throw and prevent the script from reaching the "Validation failed" path. Guard that servers is a list of dicts (similar to validate_pack()), or reuse the already-validated servers structure from the validation pass.
| for server in data.get("servers", []): | |
| servers = data.get("servers") | |
| if not isinstance(servers, list): | |
| continue | |
| for server in servers: | |
| if not isinstance(server, dict): | |
| continue |
| # Required fields | ||
| for field in ("name", "version", "description"): | ||
| if field not in pack or not pack[field]: | ||
| errors.append(f"{pack_dir_name}: missing required field '{field}' in [pack]") | ||
|
|
||
| name = pack.get("name", "") | ||
| if name and not _NAME_RE.match(name): | ||
| errors.append( | ||
| f"{pack_dir_name}: invalid pack name '{name}' — " | ||
| "must be lowercase letters, digits, and hyphens only" | ||
| ) |
There was a problem hiding this comment.
validate_pack() doesn't enforce that required scalar fields (name, version, description) are strings. If someone accidentally writes an unquoted TOML value (e.g. version = 1), the later regex checks (_NAME_RE.match(...) / _SEMVER_RE.match(...)) will raise a TypeError and crash validation. Add explicit isinstance(..., str) checks for these fields and report a validation error when the type is wrong.
- 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>
Summary
deprecated = trueand optionaldeprecated_messagein[pack]section of pack.tomlpacks/{name}.jsonandindex.jsonpacks/*.jsonwhensrc/directory is deleted (pack removal)Weave compatibility
New JSON fields are safe —
PackMetadata,PackListing, andPackReleasestructs use#[serde(default)]and silently ignore unknown fields. No Weave changes needed.Test plan
Closes #8
🤖 Generated with Claude Code