From aec61411ff727e9dbe0d2a2e464f268100a1f532 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 25 Sep 2026 02:59:21 +0000 Subject: [PATCH 1/2] Publish a lifecycle and compatibility policy, checked in CI docs/lifecycle.md is the contract for change. It covers: - package SemVer before and after 1.0 (a patch release never removes, deprecates or breaks anything); - skill SemVer with no 0.x exception, and how it meets status/update; - the deprecate -> notice -> remove path for skills, agents and formats (published in a tagged release at least 90 days before removal, 180 from 1.0), renames, and what happens to installed copies; - metadata, catalog, stamp-format and future lockfile evolution; - stable vs human CLI output, and the urgent security path. scripts/check_lifecycle.py, run by the lint job with --base, requires CHANGELOG notes, named in backticks, when compatibility changes: - a removed skill needs a Removed entry, a deprecation at the base and a published, old-enough Deprecated entry (a Security entry skips the last two); - a new deprecation needs a Deprecated entry; - an agent dropped from a skill, or a removed adapter, needs a Removed entry; - a major skill version needs a Changed entry giving the version; - a catalog schema_version change needs a Breaking entry. It also rejects a patch release with removals, deprecations or breaking entries. prepare_release.py now applies that rule before writing anything. Tests walk through a skill rename, an agent removal and a simulated stamp-format migration, pin the v1 stamp byte for byte, and show that the catalog represents every lifecycle state. No new metadata field: the notice clock comes from the CHANGELOG and release tags. Closes #78 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01HtiCGzpikMrkDYBkfQG5CX --- .github/workflows/ci.yml | 12 + CHANGELOG.md | 43 +++ CLAUDE.md | 19 +- CONTRIBUTING.md | 5 + README.md | 6 +- docs/authoring-skills.md | 9 + docs/catalog.md | 4 +- docs/compatibility.md | 4 + docs/lifecycle.md | 513 +++++++++++++++++++++++++ docs/releasing.md | 20 +- scripts/check_lifecycle.py | 552 +++++++++++++++++++++++++++ scripts/prepare_release.py | 19 +- tests/test_lifecycle.py | 687 ++++++++++++++++++++++++++++++++++ tests/test_prepare_release.py | 18 + tests/test_stamp.py | 25 +- 15 files changed, 1924 insertions(+), 12 deletions(-) create mode 100644 docs/lifecycle.md create mode 100644 scripts/check_lifecycle.py create mode 100644 tests/test_lifecycle.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ed630a4..5050a24 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -40,6 +40,18 @@ jobs: else python scripts/check_release_consistency.py fi + - name: Lifecycle notes (removals, deprecations, breaking changes) + # docs/lifecycle.md: on a PR, compatibility changes since the target + # branch need CHANGELOG entries; always, the newest release's bump + # must be big enough for what it removes or deprecates + env: + BASE_REF: ${{ github.base_ref }} + run: | + if [ -n "$BASE_REF" ]; then + uv run --locked --extra dev python scripts/check_lifecycle.py --base "origin/$BASE_REF" + else + uv run --locked --extra dev python scripts/check_lifecycle.py + fi test: runs-on: ubuntu-latest diff --git a/CHANGELOG.md b/CHANGELOG.md index dd37c62..c33fe9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -516,6 +516,49 @@ All notable changes to this project are documented here. The format is based on ### Added +- Lifecycle and compatibility policy, `docs/lifecycle.md` (#78), linked from + the README, `CONTRIBUTING.md`, `docs/releasing.md`, + `docs/authoring-skills.md`, `docs/compatibility.md` and `docs/catalog.md`. + It defines: + + - what bumps the package version before and after 1.0 (a patch release + never removes, deprecates or breaks anything); + - what makes a skill change major, minor or patch (skills don't get SemVer's + 0.x exception), and how skill versions meet `status` and `update`; + - the deprecate, notice and remove path for skills, agents and formats: + a deprecation must ship in a tagged release at least 90 days before the + removal (180 days from 1.0). A rename is a new skill plus a deprecation; + - what `status`, `update` and `uninstall` do with installed copies of a + removed skill, or of a skill that dropped an agent; + - how `meta.yaml`, the catalog, install stamps and future lockfiles (#71) + may change. Every stamp format stays readable, and `update` migrates old + stamps; + - which output is a stable contract (`catalog --json`, `provenance --json`, + run records) and which is human output that may change; + - the urgent security-fix path. + + Tests walk through a skill rename, an agent removal and a stamp-format + migration, and pin the current stamp format byte for byte. +- `scripts/check_lifecycle.py`, run by CI's `lint` job with + `--base origin/`, requires CHANGELOG notes when compatibility + changes. Each note is a bullet under `[Unreleased]` (or the newest dated + section) that names the skill, agent or adapter in backticks: + + - a removed skill needs a Removed entry, a deprecation at the base, and, once + a release has contained the skill, a published Deprecated entry at least + the notice period old. A Security entry naming the skill skips the last + two; + - a newly deprecated skill needs a Deprecated entry; + - an agent dropped from a skill, or a removed adapter, needs a Removed entry; + - a major skill version needs a Changed entry giving the new version; + - a change to the catalog's schema version needs an entry marked as + breaking. + + It also fails, as `scripts/prepare_release.py` now does before writing + anything, when the newest dated section is a patch release with Removed, + Deprecated or breaking entries (from 1.0, Removed and breaking entries need + a major release). Every error says what to add and where, and links the + policy. - Agent compatibility matrix, `docs/compatibility.md` (#79), linked from the README and `docs/adapters.md`. It lists every adapter, including the `copilot-prompt`, `cursor-rule` and `kiro-steering` legacy adapters, with: diff --git a/CLAUDE.md b/CLAUDE.md index 076c5d7..8037014 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -77,7 +77,8 @@ by `--agent all`); `skilldeck migrate` moves old-format installs to `SKILL.md`. (`tests/test_eval_scoring.py`). See `evals/README.md`. New/changed skills should be run through them. - `docs/` — `authoring-skills.md`, `adapters.md`, `catalog.md`, `compatibility.md` - (the public agent compatibility matrix), `releasing.md` + (the public agent compatibility matrix), `lifecycle.md` (the versioning, + deprecation and compatibility policy), `releasing.md` - `tests/fixtures/adapter-contracts/` — each adapter's exact rendered file, paths and env-override behaviour for one synthetic skill; checked byte for byte by `tests/test_adapter_contracts.py` @@ -140,6 +141,22 @@ by `--agent all`); `skilldeck migrate` moves old-format installs to `SKILL.md`. stay in sync — `scripts/check_release_consistency.py` enforces this in CI and `pytest`. A dated CHANGELOG section without a matching `v*` tag is prepared, not published. +- Lifecycle: follow `docs/lifecycle.md` for what bumps package/skill versions + (skills: removing a checklist area or changing output shape is major, adding + checks minor, wording/citations patch) and for deprecation → notice → removal + (a deprecation must ship in a tagged release ≥90 days before removal, 180 + from 1.0; urgent security fixes may skip it). `scripts/check_lifecycle.py` + (CI `lint` job, `--base origin/`; needs `uv run --locked --extra dev`) + fails a PR unless CHANGELOG `[Unreleased]` (or the newest dated section) has a + bullet naming the thing **in backticks**: `### Removed` for a removed skill + (which must also be deprecated at base with a published, old-enough + `### Deprecated` entry, unless a `### Security` entry names it), a removed + adapter, or an agent dropped from a skill (skill and agent in one bullet); + `### Deprecated` for a newly deprecated skill; `### Changed` naming skill + + new version for a major skill bump; `**Breaking:**` + `schema_version` for a + catalog schema bump. It also fails (as does `prepare_release.py`) when the + newest dated section is a patch release with Removed/Deprecated/**Breaking** + entries. Run it locally with `--base origin/main` before pushing. - Release CI must build once, verify wheel/sdist/plugin identity (against the tagged commit's files), produce a runtime-only SPDX SBOM and exact checksums, attest those bytes, then publish the same bundle. The build job diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0db531a..2abe8a1 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -53,6 +53,11 @@ once in an agent-neutral format — never hand-edit per-agent output. A skill's [docs/authoring-skills.md](docs/authoring-skills.md) for the full guide, and bump that skill's own `version` in `meta.yaml` whenever its content changes. +Deprecating or removing a skill, dropping an agent, or making a major version change +needs a `CHANGELOG.md` entry that names it, and CI checks for one. See +[docs/lifecycle.md](docs/lifecycle.md) for the rules and the notice period, and run +`uv run --extra dev python scripts/check_lifecycle.py --base origin/main` to check. + ## Opening the pull request - Make sure the checks above pass. diff --git a/README.md b/README.md index 24e84ef..ff34baf 100644 --- a/README.md +++ b/README.md @@ -211,6 +211,10 @@ Each skill is a directory under `src/skilldeck/skills/` containing a `meta.yaml` and a `skill.md`. See [docs/authoring-skills.md](docs/authoring-skills.md), and follow the [contributor guide](CONTRIBUTING.md) for setup and validation. -## Changelog +## Changelog and support Notable changes are recorded in [CHANGELOG.md](CHANGELOG.md). +[docs/lifecycle.md](docs/lifecycle.md) says what each kind of version bump +means, how long deprecated skills, agents and formats stay supported (a +deprecation ships in a release at least 90 days before the removal), and +what happens to skills you have already installed when one is removed. diff --git a/docs/authoring-skills.md b/docs/authoring-skills.md index 64fcd58..aedc938 100644 --- a/docs/authoring-skills.md +++ b/docs/authoring-skills.md @@ -79,6 +79,15 @@ that agent would have nothing to move to). `skilldeck list` and they write one, and `skilldeck catalog --json` reports the record to tools (see [the skill catalog](catalog.md)). +Deprecating a skill also needs a `### Deprecated` entry in `CHANGELOG.md` +naming it in backticks, and CI checks for one. The skill can be removed only +after a release has published the deprecation for the notice period (90 +days before 1.0). A rename is a new skill plus a deprecation of the old name. +See [Lifecycle and compatibility](lifecycle.md#deprecating-a-skill) for the +full path, including what happens to installed copies, and +[Skill versions](lifecycle.md#skill-versions) for which changes are major, +minor or patch. + ## `skill.md` The agent-neutral body of the skill — the actual instructions/prompt. Write it diff --git a/docs/catalog.md b/docs/catalog.md index ee5b835..1022054 100644 --- a/docs/catalog.md +++ b/docs/catalog.md @@ -77,7 +77,9 @@ from it, rather than trusting a second file that could drift from it. - `deprecated` is `null`, or an object with `since` (the skill version that first carried the deprecation), `replacement` (the skill to use instead, or `null`) and `reason`. See - [Deprecating a skill](authoring-skills.md#deprecating-a-skill). + [Deprecating a skill](authoring-skills.md#deprecating-a-skill). A removed + skill is simply absent; [Lifecycle and compatibility](lifecycle.md#the-catalog) + covers each state and how long a deprecated skill stays. `catalog` runs the full `skilldeck provenance --verify` check first. If any installed skill no longer matches its recorded digest, is missing, or has a diff --git a/docs/compatibility.md b/docs/compatibility.md index 1e55179..1c31d62 100644 --- a/docs/compatibility.md +++ b/docs/compatibility.md @@ -294,3 +294,7 @@ handles a change like this: fix as soon as it merges, following [Releasing](releasing.md), instead of batching it with other changes. Have the changelog entry tell users to run `skilldeck migrate` or `skilldeck update`. + +When skilldeck itself drops an agent or a format that the vendor still +supports, it deprecates it first and waits a notice period: see +[Removing an agent or format](lifecycle.md#removing-an-agent-or-format). diff --git a/docs/lifecycle.md b/docs/lifecycle.md new file mode 100644 index 0000000..d7f1853 --- /dev/null +++ b/docs/lifecycle.md @@ -0,0 +1,513 @@ +# Lifecycle and compatibility + +This page is skilldeck's contract for change. It says what each kind of +version bump promises, how long a deprecated skill, agent or format stays, +what happens to files you have already installed, and which output you can +build tools on. It applies to every published release, starting with the +first one. + +A CI check enforces the parts a script can check (see +[Release checks](#release-checks)). The rest is for reviewers. + +## At a glance + +| Change | Package release (before 1.0 / from 1.0) | Notice | CHANGELOG entry | +|---|---|---|---| +| Fix a skill's wording, typos or citations | patch | none | Changed or Fixed | +| Add a skill, agent, format, command, option or catalog field | patch or minor / minor | none | Added | +| Deprecate a skill, agent, format, command or option | minor / minor | none: this starts the notice period | Deprecated (checked for skills) | +| Remove a skill | minor / major | deprecation published at least 90 days earlier (180 from 1.0) (checked) | Removed (checked) | +| Remove an agent or format, or drop an agent from one skill | minor / major | the same, unless the vendor removed it first | Removed (checked) | +| Make a breaking change to a skill (a major skill version) | minor / minor | none: read the entry before `update` | Changed, with the new version (checked) | +| Change the stamp format | minor / minor | none: old stamps stay readable | Changed | +| Bump the catalog's `schema_version` | minor / major | none | **Breaking:** (checked) | +| Fix an urgent security problem | whatever the change needs | may skip all of it | Security, plus an advisory | + +## Versions + +### The package + +The package version (`pyproject.toml`, and `skilldeck --version`) follows +[SemVer](https://semver.org/). It covers skilldeck's public surface: + +- the commands, their options, defaults and exit codes; +- which skills and agents exist, by name; +- where each adapter installs, and the file it writes (the + [compatibility matrix](compatibility.md)); +- the install stamp; +- the machine-readable output listed under + [The command line](#the-command-line); +- the `meta.yaml` fields skill authors can use; +- the Python versions it runs on. + +A skill's guidance is versioned by the skill itself (see +[Skill versions](#skill-versions)), not by the package. + +**Before 1.0 (now)**, a breaking change bumps the minor version (`0.4.0` → +`0.5.0`). A deprecation also needs at least a minor release. New features and +fixes can go in either kind of release. So a patch release (`0.4.0` → +`0.4.1`) never removes, deprecates or breaks anything, and is always safe to +take. + +A change is breaking if it does any of these: + +- removes a skill, agent, format, command or option; +- moves an install location, or changes the stamp so that existing installs + are no longer recognised; +- bumps the `schema_version` of the catalog or of `provenance --json`; +- changes a default, or what an exit code means; +- drops a Python version (0.2.0 dropped 3.9). + +**From 1.0**, a breaking change needs a major release, a deprecation or a new +feature needs a minor release, and a fix needs a patch release. Dropping a +Python version counts as a minor change once that version has reached its +upstream end of life. Dropping one that upstream still supports is breaking. + +The release check holds releases to this. The newest dated CHANGELOG section +can't be a patch release if it has `### Removed` or `### Deprecated` entries, +or an entry marked **Breaking:**. From 1.0, Removed and Breaking entries need +a major release. `scripts/prepare_release.py` refuses such a version before +it writes anything. + +### Skill versions + +Each skill's `version` in `meta.yaml` is SemVer too, applied to what the skill +tells the agent to do. Skills don't get SemVer's 0.x exception: the first +breaking change to a 0.x skill takes it to 1.0.0. + +| Bump | When the change | Examples | +|---|---|---| +| major | takes something away, or changes the shape of the result | removing a checklist area or a kind of finding; narrowing what the skill reviews; handing an area to another skill in [Which skill owns what](finding-output.md#which-skill-owns-what); changing the output shape (the finding format, severity rubric or report header) | +| minor | adds to what the skill does, without taking anything away | new checks or a new checklist area; new sources that add checks; covering another kind of file; deprecating the skill; changing its `category`, which moves it between `catalog --category` filters | +| patch | leaves what it reports unchanged | wording, typos, clarifications, citation and link fixes, a clearer worked example, a new `description` | + +Adding an agent to `supported-agents` is a patch. Dropping one follows +[Dropping an agent from a skill](#dropping-an-agent-from-a-skill). + +A major skill version needs at least a minor package release. It also needs a +`### Changed` entry naming the skill and its new version (checked), saying +what changed and what users should do. + +How skill versions meet `status` and `update`: + +- Each installed file's stamp records the skill version, so + `skilldeck status` shows `1.3.0 stale (bundled: 2.0.0)` when a newer version + is bundled. +- "Stale" compares content, not versions. A file is stale whenever it differs + from what installing now would write. A skilldeck upgrade that changes how + a skill is rendered makes installs stale with the same version on both + sides. +- `skilldeck update` rewrites every stale, unedited install, whatever the size + of the bump. It never overwrites a locally modified install without + `--force`. Check the CHANGELOG for major skill versions before you run it. + To stay on the old guidance for a while, keep running the older skilldeck + release (`uvx skilldeck@X.Y.Z`), or edit the installed file, which `update` + then leaves alone. + +## Deprecating a skill + +A deprecated skill stays in the bundle, installable and listed, for the +notice period. To deprecate one: + +1. Add `deprecated` to its `meta.yaml` and bump its minor version. + [Deprecating a skill](authoring-skills.md#deprecating-a-skill) gives the + fields and the rules the loader applies: + + ```yaml + version: 1.3.0 + deprecated: + since: 1.3.0 # the skill's own version, not skilldeck's + replacement: new-review # optional + reason: Renamed to new-review. + ``` + + `reason` is required, so a skill without a replacement still says why. +2. Add a `### Deprecated` entry under `[Unreleased]` in `CHANGELOG.md` (checked). + Name the skill in backticks, give its replacement or the reason, and say + that it will be removed no earlier than 90 days after the release (180 + from 1.0). +3. Ship it in a minor release. + +Once it's released: + +- `skilldeck list` and `skilldeck catalog` mark it + `(deprecated since 1.3.0; use new-review)`; +- `install` and `update` print a warning on stderr whenever they write it; +- `skilldeck catalog --json` reports `deprecated` with `since`, `replacement` + and `reason`, for tools. + +Installed copies keep working, and `update` keeps them current. A deprecated +skill still gets fixes, security fixes included, but no new checks. New work +goes into its replacement. + +Claude Code plugin users see no warning. The plugin carries the skill +unchanged, so the CHANGELOG is where they learn about the deprecation. + +## Removing a skill + +The notice period has two parts: + +- the deprecation must ship in at least one **published** release, meaning a + dated CHANGELOG section with its `vX.Y.Z` tag; +- the removal can merge no sooner than **90 days** after that release's date + (**180 days** from 1.0). Before 1.0 it goes in a minor release, and from + 1.0 in a major one. + +Why these numbers: + +- Users see a deprecation only through a release. `list`, `install`, + `update` and the catalog report the deprecation state of the version you + run, so a deprecation that no release has published has warned nobody. +- 90 days is a quarter. A team that upgrades its tools quarterly, whether + pinned in CI or on a `uv tool upgrade` schedule, gets at least one release + that warns before the skill is gone. +- Before 1.0 the set of skills is small and still changing, so waiting longer + mostly delays the cleanup. From 1.0, 180 days gives two quarterly upgrade + cycles, and removals are collected into the rarer major releases. + +To remove the skill: + +1. Delete its directory and regenerate the plugin (`scripts/build_plugin.py`). +2. Add a `### Removed` entry naming the skill (checked). Say what replaces it + and how to delete installed copies (see below). + +The release check also requires the skill to have been deprecated at the +base of the pull request. If any release tag contains the skill, it also +requires the deprecation to have been published for the notice period. A +skill that no release ever contained must still be deprecated first, but it +has no waiting period. [Security fixes](#security-fixes) can skip all of +this. + +A removed skill is gone from `list` and from the catalog. The catalog has no +"removed" state: the CHANGELOG's Removed entry is the record, and the release +check makes sure it exists. + +### What happens to installed copies + +skilldeck never deletes a removed skill's files by itself. After you upgrade +to a release without the skill: + +- `skilldeck status --agent ` lists each leftover copy as + `orphan: ( )`; +- `skilldeck update` ignores it, so the copy stays as it was and the agent + keeps loading it; +- `skilldeck uninstall ` fails with `error: unknown skill: `, and + `uninstall --all` leaves it alone, because `uninstall` only knows bundled + skills. + +To get rid of a copy, delete it yourself (the file, and the `/` folder +it sits in for a `SKILL.md` agent), using the path `status` prints. You can +also run `skilldeck uninstall --agent ` before you upgrade, +while you still have a release that bundles the skill. Claude Code plugin +users lose the skill when the plugin next updates after the removal reaches +`main`. + +### Renaming a skill + +A rename is a new skill plus a deprecation: + +1. In one pull request, add the skill under its new name and mark the old one + `deprecated` with `replacement: `. The new skill must support + every agent the old one does, or the loader refuses the replacement. Add + an `### Added` entry for the new name and a `### Deprecated` entry for the + old one. +2. Both ship for the notice period. `skilldeck install ` still + works, and warns you to use the new name. The catalog lists the old skill + with its `replacement`, so tools can move users over. +3. Remove the old skill as described above. + +Users switch by installing the new name and uninstalling the old one. Both +commands work throughout the notice period: + +```bash +skilldeck install new-review --agent claude +skilldeck uninstall old-review --agent claude +``` + +`test_walkthrough_renaming_a_skill` in `tests/test_lifecycle.py` runs through +these steps, checking the catalog and the release check at each one. + +## Agents and formats + +skilldeck supports an agent while the vendor ships it and a primary source +confirms where it reads skills: the agent's code, a shipped binary, or the +vendor's docs. It doesn't promise to support every agent, or every agent +version, forever. The [compatibility matrix](compatibility.md) lists each +agent and format, and how each was checked. + +### Adding an agent or format + +A new agent (a native adapter) or a new legacy format is a feature. It ships +in a minor release with an `### Added` entry, the adapter's contract fixtures +and a matrix row. See [Adding a new agent](adapters.md#adding-a-new-agent). + +### When the vendor changes something + +Follow +[When a vendor changes a location or format](compatibility.md#when-a-vendor-changes-a-location-or-format). +The vendor's timeline wins. skilldeck points its adapter at the new location +as soon as a primary source confirms the move. It keeps the old format as an +opt-in legacy adapter while agent versions still in use read it, and adds it +to `MIGRATIONS` so that `skilldeck migrate --agent ` moves existing +installs. + +A format the vendor has stopped loading leaves nothing to protect. It can +stop being an install target without notice and stay only as a `migrate` +source, as happened when Codex dropped custom prompts in 0.118.0. If users +have to move their installs, the CHANGELOG entry is marked **Breaking:** and +gives the `migrate` command to run. + +### Removing an agent or format + +When skilldeck itself drops an agent or a legacy format, for example because +the product was discontinued or can no longer be checked, it follows the same +steps as for a skill: + +1. **Deprecate** the adapter in a minor release, with a `### Deprecated` + entry naming it and a note in its matrix row. +2. **Wait** for the same notice period: 90 days after the release that + published the deprecation (180 from 1.0). +3. **Remove** it from `ADAPTERS` or `LEGACY_ADAPTERS`, along with its + contract fixtures and matrix row. Add a `### Removed` entry naming it + (checked) that lists the paths it installed to. If a successor format + exists, keep the old one in `MIGRATIONS` so that `migrate` can still move + installs. + +After the removal, skilldeck rejects `--agent `, so `status` and +`uninstall` can no longer see that adapter's files. Remove them before you +upgrade, with `skilldeck uninstall --all --agent ` (once for each +scope), or delete them by hand from the paths the Removed entry lists. + +The release check requires the Removed entry. Reviewers check the +deprecation and the notice period for adapters, because a script can't tell +a skilldeck decision from a vendor-forced removal, which is exempt. + +### Dropping an agent from a skill + +Removing an agent from one skill's `supported-agents`, while skilldeck keeps +supporting the agent, removes the skill for that agent's users. It needs a +minor release before 1.0 and a major one from 1.0. Announce it first with a +`### Deprecated` entry naming the skill and the agent, and wait the same +notice period. The removal then needs a single `### Removed` entry naming +both the skill and the agent (checked). + +Installed copies for that agent stay where they are, and the agent keeps +loading them, but skilldeck stops managing them. `status --agent ` no +longer lists the skill, not even as an orphan, and `update` no longer +refreshes it. `skilldeck uninstall --agent ` still removes it, +so the Removed entry should give that command. + +`tests/test_lifecycle.py` pins this behaviour and walks through removing an +agent from one skill and removing an adapter altogether. + +## Metadata and file formats + +### `meta.yaml` + +The loader rejects any `meta.yaml` key it doesn't know, so a skill can't use +a new field until a skilldeck release knows it. Adding a field is therefore a +minor package release. That release updates the loader +(`src/skilldeck/registry.py`), [Authoring skills](authoring-skills.md) and, if +tools should see the field, the catalog, where it is an additive change. +Removing or renaming a field, or tightening a rule so that existing metadata +fails, is breaking for skill authors. + +The one lifecycle field is `deprecated`, with `since`, `reason` and an +optional `replacement`. It deliberately has no removal date or version. The +notice period starts when a release publishes the deprecation, and that date +is known only from the CHANGELOG and the release tag, which the release check +reads. A date written into `meta.yaml` in advance would be a guess that could +disagree with them. + +### The catalog + +`skilldeck catalog --json` follows the +[compatibility rules in docs/catalog.md](catalog.md#compatibility-rules): +additive changes keep `schema_version` 1, and breaking ones bump it. A bump +needs a CHANGELOG entry marked **Breaking:** that mentions `schema_version` +(checked), so it goes in a minor release before 1.0 and a major one from 1.0. + +The catalog can represent every state in a skill's lifecycle: + +| State | Catalog entry | +|---|---| +| active | `"deprecated": null` | +| deprecated, with a replacement | `"deprecated": {"since": "1.3.0", "replacement": "new-review", "reason": "..."}` | +| deprecated, with a reason only | `"deprecated": {"since": "1.3.0", "replacement": null, "reason": "..."}` | +| removed | not in `skills`; the CHANGELOG records the removal | + +A tool that remembers skills from an earlier catalog should treat a name that +disappears as removed, and look up its replacement in the last catalog that +listed it. `tests/test_lifecycle.py` checks each state against the published +schema. + +### Install stamps + +Every installed file ends with one stamp line: + +``` + +``` + +The format has no version marker. It is version 1 by its shape, and +`skilldeck.stamp.parse` reads only that shape. The stamp is how `status`, +`update`, `install` and `uninstall` recognise skilldeck's own unedited files, +so a file whose stamp skilldeck can't read counts as one it didn't write: +`update` skips it, and `install` and `uninstall` refuse it without `--force`. +That already happened once, to the unstamped files of skilldeck 0.3.0 and +earlier. + +So a change to the stamp format follows these rules: + +1. **Keep reading every format skilldeck has written.** No release drops one. + Each costs one pattern. +2. **Mark the new format**, for example ``, + so that it can't be mistaken for version 1. A stamp with no marker stays + version 1. +3. **Let `update` do the migration.** An unedited file with a version 1 stamp + no longer matches what installing writes, so `status` shows it as stale and + `update` rewrites it with the new stamp. No migration command and no + `--force` are needed. An edited file shows as "modified locally" and is + left alone, as it is today. +4. **Keep `hash=` meaning the same thing**: the SHA-256 of the file above the + stamp, which the catalog's `rendered_sha256` equals. Changing that would + also be a breaking catalog change. +5. **Record it** with a `### Changed` entry, and update the adapter contract + fixtures, since the stamp is part of every expected file. That changes the + compatibility digest too. + +A stamp-format change that follows these rules is a minor release. +Downgrading skilldeck across one isn't supported: the older release reads the +new stamp as no stamp, and needs `install --force` to take the files back. + +`tests/test_stamp.py` pins the version 1 format byte for byte. +`test_walkthrough_stamp_format_migration` in `tests/test_lifecycle.py` +simulates a second format and checks that `status` and `update` migrate files +as described here. + +### Lockfiles and install state + +Today the install state is the stamped files themselves. There is no lockfile +or install database. [#71](https://github.com/IcebergAI/skilldeck/issues/71) +will add lockfiles and bundles, and they will follow this page: + +- A lockfile carries its own integer `schema_version`, under the catalog's + additive and breaking rules. Like stamps, every lockfile version skilldeck + has written stays readable. +- It records identities the catalog already publishes: the skill's name, + version and `canonical_sha256`, plus the adapter, the scope and the + `rendered_sha256` for each agent (the stamp's `hash=`). `rendered_sha256` + can change with the rendering while the skill version stays the same, so a + lockfile that pins it is also pinned to a skilldeck version. +- A locked skill that becomes deprecated keeps installing, with the usual + warning, through the notice period. A locked skill, agent or format that + has been removed fails before anything is written, with an error that names + it and points to the CHANGELOG. + +Lockfiles aren't a stable contract until they ship under these rules. + +## The command line + +These are stable and covered by the package version: + +- command and option names, arguments, defaults, and the `--agent` and + `--scope` values; +- exit codes: 0 for success, 1 when anything failed, 2 for a usage error; +- install locations and file contents, as pinned by the + [adapter contracts](compatibility.md#contract-tests); +- `skilldeck catalog --json` and `--schema` (see [the catalog](catalog.md)); +- `skilldeck provenance --json` (`schema_version` 1), under the same additive + and breaking rules as the catalog; +- eval run records (`evals/run-record.schema.json`, `schema_version` 1), + under the same rules. They belong to the repository's eval tooling rather + than the package. + +Human-readable output isn't stable, and can change in any release without +notice. That covers `list`, `status`, `update`, `install`, `uninstall`, +`migrate`, `show`, plain `catalog` and `provenance`, and the wording of every +warning and error. Use `--json` where it exists. `status` has no +machine-readable form yet, so don't parse it. + +To remove or rename a command or option, deprecate it first: its help text +says so, and using it prints a warning. Then wait the same notice period. A +renamed option keeps its old spelling as a deprecated alias for that time. + +## Security fixes + +The urgent path is for a vulnerability in skilldeck's code or releases, or +for a skill whose guidance is itself harmful, such as one telling an agent to +weaken a security control. + +It may skip: + +- the deprecation and the notice period, so a harmful skill can be changed or + removed straight away, and so can an adapter; +- batching: the fix is released as soon as it merges, not with the next + planned release. + +It must still: + +- go through the normal release: the release checks, the tag and the gated + release workflow; +- bump versions as SemVer requires, so a removal still needs a minor release + before 1.0; +- add a `### Security` entry that names what is affected in backticks, gives + the affected versions, and says what users should do (for example + `skilldeck update`, or `skilldeck uninstall --agent `), along + with the usual Removed or Changed entry. For a skill removal, the Security + entry naming the skill is what lets the release check skip the deprecation + and notice requirements; +- for a vulnerability in skilldeck itself, publish a GitHub security advisory, + handling the report as [SECURITY.md](../SECURITY.md) describes. + +Fixes ship on the latest release only, with no backports. For a confirmed +high- or critical-severity issue, the aim is a release within 7 days. + +## Release checks + +`scripts/check_lifecycle.py` runs in CI's `lint` job on every pull request as +`--base origin/`, comparing the pull request with its target. +It fails unless `CHANGELOG.md` has: + +- for a **removed skill**, a `### Removed` entry naming it. Unless a + `### Security` entry names it too, the skill must have been deprecated at + the base. If a release tag contains the skill, a `### Deprecated` entry + naming it must also be in a tagged, dated section at least 90 days old (180 + from 1.0); +- for a **newly deprecated skill**, a `### Deprecated` entry naming it; +- for an **agent dropped from a skill's `supported-agents`**, a single + `### Removed` entry naming both the skill and the agent. If the agent's + adapter is gone altogether, the next rule covers it instead; +- for a **removed adapter** (listed in the base's + `tests/fixtures/adapter-contracts/contracts.json` but no longer in + `ALL_ADAPTERS`), a `### Removed` entry naming it; +- for a **major skill version**, a `### Changed`, `### Removed` or + `### Security` entry naming the skill and giving its new version; +- for a **catalog `schema_version` change**, an entry marked **Breaking:** + that mentions `schema_version`. + +Entries count only under `## [Unreleased]` or in the newest dated section, +which is where cutting a release moves them. They must name each skill, +agent or adapter in backticks, like `` `old-review` ``. Every error says what +is missing, where to add it, and which section of this page applies. + +With or without `--base`, the script also applies the version-bump rule from +[The package](#the-package) to the newest dated section. On a push to `main` +and under `pytest` it runs that rule on the real CHANGELOG, and +`tests/test_lifecycle.py` tests every rule on small synthetic repositories. +To run it yourself before pushing: + +```bash +uv run --locked --extra dev python scripts/check_lifecycle.py --base origin/main +``` + +These stay with reviewers: + +- whether a skill change is major, minor or patch (the check only sees the + number); +- the deprecation and notice period for removing an adapter, a format, a + command or an option, or dropping an agent from a skill, where a vendor's + move can make the notice moot; +- whether an entry actually gives the replacement and the commands to run; +- the stamp-format rules; +- whether a fix qualifies for the urgent security path. diff --git a/docs/releasing.md b/docs/releasing.md index 2475bad..965632d 100644 --- a/docs/releasing.md +++ b/docs/releasing.md @@ -11,10 +11,14 @@ rules can't silently drift. - **SemVer**, and the project is **pre-1.0**: a breaking change bumps the **minor** (`0.2 → 0.3`); features and fixes bump the minor or patch at discretion. (Dropping Python 3.9 in 0.2.0 was breaking; the two new skills in - 0.3.0 were additive.) + 0.3.0 were additive.) A release with Removed or Deprecated entries, or an + entry marked **Breaking:**, can't be a patch release. See + [Lifecycle and compatibility](lifecycle.md#the-package) for what counts as + breaking, the rules from 1.0, and the notice period before a removal. - **Skill versions are independent.** Each skill carries its own `version` in `meta.yaml`; bump it whenever that skill's content changes, regardless of the - project version. + project version. [Skill versions](lifecycle.md#skill-versions) says which + changes are major, minor or patch. - **The Claude Code plugin version follows its content.** It equals the project version only for the content prepared for that release; any other content on `main` ships as a development version such as @@ -179,6 +183,18 @@ Consumer verification is documented in example `refs/tags/x/v0.3.0`, `v0.3.0rc1`, or `v0.04.0`, which PEP 440 would publish as 0.4.0) is rejected outright. +`scripts/check_lifecycle.py` checks the lifecycle notes described in +[Lifecycle and compatibility](lifecycle.md#release-checks). It needs PyYAML +and the package, so the `lint` job runs it with `uv run`. On a pull request +it compares the change with `--base origin/`, and requires a +CHANGELOG entry for each removed or newly deprecated skill, agent dropped +from a skill, removed adapter, major skill version and catalog +`schema_version` change. A skill removal must also follow a published +deprecation by the notice period. Every run also checks that the newest +dated CHANGELOG section is a big enough version bump for its Removed, +Deprecated and **Breaking:** entries, which `prepare_release.py` checks +before it writes anything. + Every script reads the version through `scripts/_pyproject.py`, which takes `version` from the `[project]` table only (via `tomllib` on Python 3.11+, and a `[project]`-scoped scan on 3.10), so a `version` key in another table can never diff --git a/scripts/check_lifecycle.py b/scripts/check_lifecycle.py new file mode 100644 index 0000000..f2e6d20 --- /dev/null +++ b/scripts/check_lifecycle.py @@ -0,0 +1,552 @@ +#!/usr/bin/env python3 +"""Require CHANGELOG lifecycle notes when a change affects compatibility. + +``docs/lifecycle.md`` is the policy; this script checks the parts of it that a +machine can. With ``--base `` (a PR's target branch; CI's ``lint`` job +passes ``origin/``), it compares the working tree with that ref: + +- a skill directory removed: a ``### Removed`` entry naming the skill. Unless a + ``### Security`` entry names it too (the urgent path), the skill must be + ``deprecated`` in its ``meta.yaml`` at the base and, if any release tag + contains the skill, a ``### Deprecated`` entry naming it must sit in a + published release (a dated section whose ``v`` tag exists) dated at + least ``NOTICE_DAYS`` ago (``NOTICE_DAYS_STABLE`` from 1.0); +- a skill newly ``deprecated``: a ``### Deprecated`` entry naming it; +- a skill no longer listing an agent in ``supported-agents``: a ``### Removed`` + entry naming the skill and the agent (an agent whose adapter is gone + altogether is covered by the next rule instead); +- an adapter removed (named in the base's adapter contracts, missing from + ``ALL_ADAPTERS`` now): a ``### Removed`` entry naming it; +- a skill's major version raised: a ``### Changed``, ``### Removed`` or + ``### Security`` entry naming the skill and its new version; +- the catalog's ``schema_version`` changed: a ``**Breaking:**`` entry that + mentions ``schema_version``. + +Each entry must be a bullet in ``## [Unreleased]`` or in the newest dated +section (cutting a release moves ``[Unreleased]`` there), and must name the +skill, agent or adapter in backticks, e.g. `` `old-review` ``. + +With or without ``--base`` it also checks the newest dated CHANGELOG section's +version against the one before it: a section with ``### Removed`` or +``### Deprecated`` entries, or an entry marked ``**Breaking``, must not be a +patch release (and, from 1.0, Removed or Breaking needs a major). + +Needs PyYAML and the ``skilldeck`` package from this checkout, so run it with +``uv run --locked --extra dev python scripts/check_lifecycle.py``. +""" + +from __future__ import annotations + +import argparse +import datetime +import json +import re +import subprocess +import sys +from collections.abc import Iterable +from dataclasses import dataclass, field +from pathlib import Path + +import yaml + +ROOT = Path(__file__).resolve().parent.parent +sys.path.insert(0, str(ROOT / "src")) # run from a checkout without installing +sys.path.insert(0, str(ROOT / "scripts")) + +import _pyproject # noqa: E402 +import check_release_consistency as consistency # noqa: E402 + +from skilldeck.adapters import ALL_ADAPTERS # noqa: E402 + +#: minimum days between the release that announces a removal and the +#: removal: before 1.0, and from 1.0 (see docs/lifecycle.md#removing-a-skill) +NOTICE_DAYS = 90 +NOTICE_DAYS_STABLE = 180 +SKILLS_PATH = "src/skilldeck/skills" +CONTRACTS_PATH = "tests/fixtures/adapter-contracts/contracts.json" +CATALOG_SCHEMA_PATH = "src/skilldeck/catalog.schema.json" +POLICY = "docs/lifecycle.md" + +_SECTION_RE = re.compile( + r"## \[(?PUnreleased|\d+\.\d+\.\d+)\]" + r"(?:\s*-\s*(?P\d{4}-\d{2}-\d{2}))?" +) +_GROUP_RE = re.compile(r"^### +(.+?)\s*$", re.MULTILINE) +_ENTRY_RE = re.compile(r"^[-*] ", re.MULTILINE) + + +# --- the CHANGELOG ------------------------------------------------------------ + + +@dataclass +class Section: + """One ``## [...]`` section: its entries, by ``### Group``.""" + + version: str | None # None for [Unreleased] + date: datetime.date | None + groups: dict[str, list[str]] = field(default_factory=dict) + + def entries(self, *groups: str) -> list[str]: + """The bullets under ``groups`` (all of them when none are given).""" + names = groups or tuple(self.groups) + return [entry for name in names for entry in self.groups.get(name, [])] + + +def parse_changelog(text: str) -> list[Section]: + """Every ``## [Unreleased]`` or ``## [x.y.z]`` section, in file order. + + Entries are the top-level ``-`` bullets, each with its indented + continuation lines and sub-bullets. Group names are title-cased, so + ``### removed`` counts as ``Removed``. + """ + sections: list[Section] = [] + text = text.replace("\r\n", "\n") + for chunk in re.split(r"^(?=## \[)", text, flags=re.MULTILINE): + header = _SECTION_RE.match(chunk) + if not header: + continue # the preamble, or a heading this parser does not know + version = header.group("version") + date = header.group("date") + section = Section( + version=None if version == "Unreleased" else version, + date=datetime.date.fromisoformat(date) if date else None, + ) + parts = _GROUP_RE.split(chunk) + # parts: [before the first group, name, body, name, body, ...] + for name, body in zip(parts[1::2], parts[2::2], strict=True): + entries = [e.strip() for e in _ENTRY_RE.split(body)[1:] if e.strip()] + section.groups.setdefault(name.strip().title(), []).extend(entries) + sections.append(section) + return sections + + +def recent_sections(sections: list[Section]) -> list[Section]: + """``[Unreleased]`` and the newest dated section. + + A note belongs in ``[Unreleased]``; cutting a release + (``scripts/prepare_release.py``) moves it into a new dated section and + leaves ``[Unreleased]`` empty, so the newest dated section counts too. + """ + recent = [s for s in sections if s.version is None][:1] + dated = _dated(sections) + return [*recent, *dated[-1:]] + + +def _dated(sections: list[Section]) -> list[Section]: + """The dated ``## [x.y.z] - DATE`` sections, oldest version first.""" + return sorted( + (s for s in sections if s.version is not None and s.date is not None), + key=lambda s: consistency.version_key(s.version or "0.0.0"), + ) + + +def names(entry: str, *terms: str) -> bool: + """Whether ``entry`` names every one of ``terms`` in backticks.""" + return all(f"`{term}`" in entry for term in terms) + + +def mentions_version(entry: str, version: str) -> bool: + """Whether ``entry`` gives ``version`` as a whole version, not part of one + (``2.0.0`` is not in ``12.0.0`` or ``2.0.01``; a full stop may follow).""" + pattern = rf"(? bool: + return any( + names(entry, *terms) + for section in recent_sections(sections) + for entry in section.entries(*groups) + ) + + +# --- skills, adapters and the catalog at a ref or in the working tree --------- + + +@dataclass(frozen=True) +class SkillFacts: + """The parts of a skill's ``meta.yaml`` the lifecycle rules look at.""" + + version: str | None + agents: frozenset[str] + deprecated: bool + + +def skill_facts(meta_text: str) -> SkillFacts: + """Read ``meta_text`` leniently: the registry validates it elsewhere.""" + try: + data = yaml.safe_load(meta_text) + except yaml.YAMLError: + data = None + if not isinstance(data, dict): + return SkillFacts(version=None, agents=frozenset(), deprecated=False) + version = data.get("version") + agents = data.get("supported-agents") + return SkillFacts( + version=version if isinstance(version, str) else None, + agents=frozenset(agent for agent in (agents or []) if isinstance(agent, str)) + if isinstance(agents, list) + else frozenset(), + deprecated=isinstance(data.get("deprecated"), dict), + ) + + +def _git(*args: str) -> subprocess.CompletedProcess[bytes]: + return subprocess.run( + ["git", "-C", str(ROOT), *args], capture_output=True, check=False + ) + + +def _git_text(ref: str, path: str) -> str | None: + """``path`` as committed at ``ref``, or None if it is not there.""" + if _git("cat-file", "-e", f"{ref}:{path}").returncode != 0: + return None + return _git("show", f"{ref}:{path}").stdout.decode("utf-8", "replace") + + +def skills_at(ref: str) -> dict[str, SkillFacts]: + """Each skill directory with a ``meta.yaml`` at ``ref``.""" + listing = _git("ls-tree", "-z", "--name-only", ref, "--", f"{SKILLS_PATH}/") + skills = {} + for path in listing.stdout.decode("utf-8").split("\0"): + name = path.rpartition("/")[2] + if not name or name.startswith("."): + continue + meta = _git_text(ref, f"{path}/meta.yaml") + if meta is not None: + skills[name] = skill_facts(meta) + return skills + + +def skills_now() -> dict[str, SkillFacts]: + """Each skill directory with a ``meta.yaml`` in the working tree.""" + root = ROOT / SKILLS_PATH + if not root.is_dir(): + return {} + return { + meta.parent.name: skill_facts(meta.read_text(encoding="utf-8")) + for meta in sorted(root.glob("*/meta.yaml")) + if not meta.parent.name.startswith(".") + } + + +def adapters_at(ref: str) -> set[str] | None: + """The adapter names in the adapter contracts at ``ref``. + + ``tests/test_adapter_contracts.py`` keeps that file's adapters equal to + ``ALL_ADAPTERS``, so it records which adapters the base shipped without + importing the base's code. None if the base predates the contracts. + """ + text = _git_text(ref, CONTRACTS_PATH) + if text is None: + return None + try: + adapters = json.loads(text)["adapters"] + except (json.JSONDecodeError, KeyError, TypeError): + return None + return set(adapters) if isinstance(adapters, dict) else None + + +def _schema_version(text: str | None) -> object: + if text is None: + return None + try: + return json.loads(text)["properties"]["schema_version"]["const"] + except (json.JSONDecodeError, KeyError, TypeError): + return None + + +def _release_tags() -> list[str]: + """Every exact ``vX.Y.Z`` tag (as a full ref).""" + refs = _git("for-each-ref", "--format=%(refname)", "refs/tags/v*").stdout + tags = [] + for ref in refs.decode("utf-8").split(): + try: + consistency.normalize_tag(ref) + except ValueError: + continue + tags.append(ref) + return tags + + +def _tag_exists(version: str) -> bool: + ref = f"refs/tags/v{version}^{{commit}}" + return _git("rev-parse", "-q", "--verify", ref).returncode == 0 + + +def released(name: str) -> bool: + """Whether any ``vX.Y.Z`` release tag contains skill ``name``.""" + path = f"{SKILLS_PATH}/{name}/meta.yaml" + return any( + _git("cat-file", "-e", f"{tag}:{path}").returncode == 0 + for tag in _release_tags() + ) + + +# --- the rules ----------------------------------------------------------------- + + +def _where(group: str, *terms: str) -> str: + named = " and ".join(f"`{term}`" for term in terms) + article, named = ("one", f"both {named}") if len(terms) > 1 else ("a", named) + return ( + f"add {article} `### {group}` entry naming {named} (in backticks) under " + "`## [Unreleased]` in CHANGELOG.md" + ) + + +def notice_days() -> int: + """The notice period for the project version (the stricter one if the + version cannot be read).""" + try: + major = consistency.version_key(_pyproject.project_version(ROOT))[0] + except (_pyproject.PyprojectError, ValueError): + return NOTICE_DAYS_STABLE + return NOTICE_DAYS if major == 0 else NOTICE_DAYS_STABLE + + +def _notice_error( + name: str, sections: list[Section], today: datetime.date, days: int +) -> str: + """Why removing skill ``name`` today breaks the notice rule, or ``""``.""" + announced = [ + section + for section in sections + if section.version is not None + and section.date is not None + and any(names(entry, name) for entry in section.entries("Deprecated")) + and _tag_exists(section.version) + ] + if not announced: + return ( + f"skill `{name}` was removed, but no published release announces " + f"its deprecation: a `### Deprecated` entry naming `{name}` must " + f"ship in a tagged release at least {days} days before the " + f"removal ({POLICY}#removing-a-skill)" + ) + first = min(announced, key=lambda s: s.date or today) + assert first.date is not None and first.version is not None + allowed = first.date + datetime.timedelta(days=days) + if today < allowed: + return ( + f"skill `{name}` was removed too early: its deprecation was " + f"published in {first.version} on {first.date.isoformat()}, so it " + f"can be removed from {allowed.isoformat()} ({days} days' notice; " + f"{POLICY}#removing-a-skill)" + ) + return "" + + +def _major(version: str) -> int: + """The MAJOR of ``version``; -1 if it is not MAJOR.MINOR.PATCH (the + registry rejects that, so no major bump is inferred from it).""" + try: + return consistency.version_key(version)[0] + except ValueError: + return -1 + + +def skill_errors( + base: dict[str, SkillFacts], + head: dict[str, SkillFacts], + sections: list[Section], + adapters: Iterable[str], + today: datetime.date, + days: int = NOTICE_DAYS, +) -> list[str]: + """Lifecycle notes missing for the skill changes from ``base`` to ``head``, + with ``days`` of notice for a removal.""" + adapters = set(adapters) + errors = [] + for name in sorted(base.keys() - head.keys()): + if not _recorded(sections, ("Removed",), name): + errors.append( + f"skill `{name}` was removed: {_where('Removed', name)}, saying " + "what replaces it and how to delete installed copies " + f"({POLICY}#removing-a-skill)" + ) + if _recorded(sections, ("Security",), name): + continue # the urgent path: no deprecation or notice period + if not base[name].deprecated: + errors.append( + f"skill `{name}` was removed without being deprecated first: " + "mark it `deprecated` in its meta.yaml, release that, and " + f"remove it after the notice period ({POLICY}#removing-a-skill). " + f"An urgent security removal instead needs a `### Security` " + f"entry naming `{name}` ({POLICY}#security-fixes)" + ) + elif released(name): + notice = _notice_error(name, sections, today, days) + if notice: + errors.append(notice) + for name in sorted(base.keys() & head.keys()): + old, new = base[name], head[name] + if old.version is None or new.version is None: + continue # unreadable meta.yaml: the registry's tests report it + newly_deprecated = new.deprecated and not old.deprecated + if newly_deprecated and not _recorded(sections, ("Deprecated",), name): + errors.append( + f"skill `{name}` is newly deprecated: " + f"{_where('Deprecated', name)}, naming its replacement or the " + f"reason ({POLICY}#deprecating-a-skill)" + ) + for agent in sorted(old.agents - new.agents): + if agent not in adapters: + continue # the adapter itself is gone: adapter_errors covers it + if not _recorded(sections, ("Removed",), name, agent): + errors.append( + f"skill `{name}` no longer supports `{agent}`: " + f"{_where('Removed', name, agent)}, " + f"telling users to run `skilldeck uninstall {name} --agent " + f"{agent}` ({POLICY}#dropping-an-agent-from-a-skill)" + ) + if _major(new.version) > _major(old.version) and not any( + names(entry, name) and mentions_version(entry, new.version) + for section in recent_sections(sections) + for entry in section.entries("Changed", "Removed", "Security") + ): + errors.append( + f"skill `{name}` went from {old.version} to {new.version}, a " + "major (breaking) change: add a `### Changed` entry naming " + f"`{name}` and {new.version} under `## [Unreleased]` in " + "CHANGELOG.md, saying what changed and what users should do " + f"({POLICY}#skill-versions)" + ) + return errors + + +def adapter_errors( + base: set[str] | None, head: Iterable[str], sections: list[Section] +) -> list[str]: + """Lifecycle notes missing for adapters removed since ``base``.""" + if base is None: + return [] + return [ + f"adapter `{name}` was removed: {_where('Removed', name)}, listing the " + "paths it installed to so users can delete what is left " + f"({POLICY}#removing-an-agent-or-format)" + for name in sorted(base - set(head)) + if not _recorded(sections, ("Removed",), name) + ] + + +def catalog_errors(base: object, head: object, sections: list[Section]) -> list[str]: + """A catalog ``schema_version`` change needs a **Breaking:** entry.""" + if base is None or head is None or base == head: + return [] + if any( + "**Breaking" in entry and "`schema_version`" in entry + for section in recent_sections(sections) + for entry in section.entries() + ): + return [] + return [ + f"the catalog schema_version changed from {base} to {head}: add a " + "`**Breaking:**` entry mentioning `schema_version` under " + "`## [Unreleased]` in CHANGELOG.md, saying what consumers must change " + "(docs/catalog.md#compatibility-rules)" + ] + + +def release_bump_errors(sections: list[Section]) -> list[str]: + """The newest dated section must be a big enough version bump. + + Removals and **Breaking** changes need a minor release before 1.0 and a + major one after; deprecations need at least a minor release. So a patch + release is always safe to take. + """ + dated = _dated(sections) + if len(dated) < 2: + return [] + previous, newest = dated[-2], dated[-1] + assert previous.version is not None and newest.version is not None + old = consistency.version_key(previous.version) + new = consistency.version_key(newest.version) + kinds = { + "### Removed": bool(newest.entries("Removed")), + "**Breaking**": any("**Breaking" in e for e in newest.entries()), + "### Deprecated": bool(newest.entries("Deprecated")), + } + breaking = kinds["### Removed"] or kinds["**Breaking**"] + if new[0] > old[0]: + return [] + if breaking and new[0] > 0: + needed = f"a major release ({new[0] + 1}.0.0)" + elif (breaking or kinds["### Deprecated"]) and new[:2] == old[:2]: + needed = f"a minor release ({new[0]}.{new[1] + 1}.0)" + else: + return [] + found = ", ".join(kind for kind, present in kinds.items() if present) + return [ + f"CHANGELOG.md's newest release {newest.version} follows " + f"{previous.version} but has {found} entries, which need {needed}: " + f"prepare that version instead ({POLICY}#the-package)" + ] + + +def lifecycle_errors(base: str, today: datetime.date | None = None) -> list[str]: + """Every lifecycle note the change from ``base`` to the working tree lacks.""" + if today is None: + today = datetime.datetime.now(datetime.timezone.utc).date() + try: + in_git = _git("rev-parse", "--git-dir").returncode == 0 + except OSError: + in_git = False + if not in_git: + return [f"--base {base} needs a git checkout"] + if _git("rev-parse", "-q", "--verify", f"{base}^{{commit}}").returncode != 0: + return [f"base ref {base!r} not found (fetch it first)"] + sections = parse_changelog((ROOT / "CHANGELOG.md").read_text(encoding="utf-8")) + head_schema = ROOT / CATALOG_SCHEMA_PATH + return [ + *skill_errors( + skills_at(base), + skills_now(), + sections, + ALL_ADAPTERS, + today, + notice_days(), + ), + *adapter_errors(adapters_at(base), ALL_ADAPTERS, sections), + *catalog_errors( + _schema_version(_git_text(base, CATALOG_SCHEMA_PATH)), + _schema_version( + head_schema.read_text(encoding="utf-8") + if head_schema.is_file() + else None + ), + sections, + ), + ] + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--base", + help="git ref this change will merge into (a PR's target branch): " + "require CHANGELOG notes for the compatibility changes since it", + ) + args = parser.parse_args(argv) + + changelog = (ROOT / "CHANGELOG.md").read_text(encoding="utf-8") + errors = release_bump_errors(parse_changelog(changelog)) + if args.base: + errors += lifecycle_errors(args.base) + if errors: + for error in errors: + print(f"error: {error}", file=sys.stderr) + print( + f"\nSee {POLICY}: a compatibility change needs a CHANGELOG entry " + "naming what\nchanged (in backticks), under [Unreleased] or the " + "newest dated section.", + file=sys.stderr, + ) + return 1 + target = f"the changes since {args.base}" if args.base else "the newest release" + print(f"ok: lifecycle notes cover {target}") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/prepare_release.py b/scripts/prepare_release.py index bbecc8c..3f76cb2 100644 --- a/scripts/prepare_release.py +++ b/scripts/prepare_release.py @@ -16,12 +16,14 @@ Every check that can reject the release (a canonical ``X.Y.Z`` version newer than both the current one and the newest dated CHANGELOG section, an ``[Unreleased]`` section with at least one entry, no existing section for the -version) runs before any file is written. If ``uv lock`` or generating the -plugin tree fails, ``pyproject.toml``, ``CHANGELOG.md`` and ``uv.lock`` are -restored and the script exits non-zero, as they are if the regenerated -plugin would not carry exactly the release version. Only a failure while -writing the plugin tree itself, or of the final consistency guard (which the -checks above exist to prevent), leaves the edits in place for inspection. +version, and a bump big enough for the section's Removed, Deprecated and +**Breaking** entries, per docs/lifecycle.md) runs before any file is written. +If ``uv lock`` or generating the plugin tree fails, ``pyproject.toml``, +``CHANGELOG.md`` and ``uv.lock`` are restored and the script exits non-zero, +as they are if the regenerated plugin would not carry exactly the release +version. Only a failure while writing the plugin tree itself, or of the final +consistency guard (which the checks above exist to prevent), leaves the edits +in place for inspection. It does not commit, push, or tag: review the diff, open a ``Release x.y.z`` PR, and tag ``vX.Y.Z`` after the merge (which publishes to PyPI). @@ -40,6 +42,7 @@ import _pyproject # noqa: E402 import build_plugin # noqa: E402 +import check_lifecycle as lifecycle # noqa: E402 import check_release_consistency as consistency # noqa: E402 VERSION_RE = consistency.RELEASE_VERSION_RE @@ -99,6 +102,10 @@ def plan(version: str, today: str, root: Path = ROOT) -> tuple[str, dict[Path, s changelog = root / "CHANGELOG.md" new_pyproject, old = bump_pyproject(pyproject.read_text(encoding="utf-8"), version) new_changelog = cut_changelog(changelog.read_text(encoding="utf-8"), version, today) + # removals, deprecations and breaking changes need a minor (or major) bump + bump = lifecycle.release_bump_errors(lifecycle.parse_changelog(new_changelog)) + if bump: + raise SystemExit(f"error: {bump[0]}") return old, {pyproject: new_pyproject, changelog: new_changelog} diff --git a/tests/test_lifecycle.py b/tests/test_lifecycle.py new file mode 100644 index 0000000..6027c77 --- /dev/null +++ b/tests/test_lifecycle.py @@ -0,0 +1,687 @@ +"""The lifecycle policy (docs/lifecycle.md) and its release check. + +``scripts/check_lifecycle.py`` requires CHANGELOG notes for compatibility +changes. Most tests here build a small git repository whose base commit is +``main`` and edit its working tree, as a pull request would. The walkthroughs +at the end follow docs/lifecycle.md: a skill rename, an agent removal and a +stamp-format migration, plus what installed copies look like afterwards. +""" + +import datetime +import importlib.util +import json +import os +import re +import shutil +import subprocess +import sys +from pathlib import Path + +import pytest +from click.testing import CliRunner + +from skilldeck import __version__, registry +from skilldeck.adapters import ADAPTERS, ALL_ADAPTERS +from skilldeck.adapters import base as adapter_base +from skilldeck.catalog import build_catalog +from skilldeck.cli import cli +from skilldeck.provenance import content_manifest +from skilldeck.registry import discover_skills +from test_catalog import schema_errors + +_ROOT = Path(__file__).resolve().parent.parent +_SCRIPT = _ROOT / "scripts" / "check_lifecycle.py" +_spec = importlib.util.spec_from_file_location("check_lifecycle", _SCRIPT) +assert _spec and _spec.loader +check = importlib.util.module_from_spec(_spec) +sys.modules[_spec.name] = check # dataclasses look their module up there +_spec.loader.exec_module(check) + +SKILLS = check.SKILLS_PATH +TODAY = datetime.date(2026, 9, 1) + + +def _in_git_checkout() -> bool: + try: + probe = subprocess.run( + ["git", "-C", str(_ROOT), "rev-parse", "--verify", "HEAD"], + capture_output=True, + check=False, + ) + except OSError: + return False + return probe.returncode == 0 + + +# --- the live guards ------------------------------------------------------------ + + +def test_the_newest_release_is_a_big_enough_bump(): + assert check.main([]) == 0 + + +@pytest.mark.skipif(not _in_git_checkout(), reason="needs a git checkout") +def test_the_working_tree_has_the_notes_it_needs_against_head(capsys): + # exercises the git plumbing against the real repository + assert check.main(["--base", "HEAD"]) == 0, capsys.readouterr().err + + +def test_the_check_reads_the_real_adapter_contracts(): + contracts = json.loads((_ROOT / check.CONTRACTS_PATH).read_text(encoding="utf-8")) + assert set(contracts["adapters"]) == set(ALL_ADAPTERS) + schema = (_ROOT / check.CATALOG_SCHEMA_PATH).read_text(encoding="utf-8") + assert check._schema_version(schema) == 1 + + +def _anchors(doc: Path) -> set[str]: + """GitHub's heading anchors in ``doc``.""" + headings = re.findall(r"^#+ (.+)$", doc.read_text(encoding="utf-8"), re.M) + return { + re.sub(r"[^a-z0-9 -]", "", heading.lower()).replace(" ", "-") + for heading in headings + } + + +def test_every_page_the_check_links_to_exists(): + source = _SCRIPT.read_text(encoding="utf-8") + links = set(re.findall(r"(docs/[a-z-]+\.md)#([a-z0-9-]+)", source)) + links |= { + (check.POLICY, anchor) for anchor in re.findall(r"POLICY}#([a-z0-9-]+)", source) + } + assert len(links) >= 8 + for page, anchor in sorted(links): + assert anchor in _anchors(_ROOT / page), f"{page}#{anchor}" + + +# --- parsing the CHANGELOG ------------------------------------------------------- + +CHANGELOG = """\ +# Changelog + +Intro text with `old-review` in it, which is not an entry. + +## [Unreleased] + +### Removed + +- `old-review`: use `new-review`, which also covers X. Delete leftovers + with the paths `skilldeck status` prints. + - a nested bullet naming `nested-name` +- `other` goes too. + +### deprecated + +- `legacy-review` + +## [0.3.0] - 2026-06-27 + +### Added + +- `just-released` + +## [0.10.0] - 2026-07-01 + +### Changed + +- `newest-by-number` + +## [0.1.0] + +### Added + +- `undated` +""" + + +def test_parse_changelog_splits_sections_groups_and_entries(): + sections = check.parse_changelog(CHANGELOG.replace("\n", "\r\n")) + assert [(s.version, s.date) for s in sections] == [ + (None, None), + ("0.3.0", datetime.date(2026, 6, 27)), + ("0.10.0", datetime.date(2026, 7, 1)), + ("0.1.0", None), + ] + unreleased = sections[0] + assert unreleased.entries("Removed") == [ + "`old-review`: use `new-review`, which also covers X. Delete leftovers\n" + " with the paths `skilldeck status` prints.\n" + " - a nested bullet naming `nested-name`", + "`other` goes too.", + ] + assert unreleased.entries("Deprecated") == ["`legacy-review`"] # title-cased + # [Unreleased] and the newest dated section by number, not file order + assert [s.version for s in check.recent_sections(sections)] == [None, "0.10.0"] + + +def test_names_needs_every_term_in_backticks(): + entry = "`old-review` no longer supports `codex`; old-review lives on" + assert check.names(entry, "old-review", "codex") + assert not check.names(entry, "old-review", "claude") + assert not check.names("old-review was removed", "old-review") + assert not check.names("`old-review-2` was removed", "old-review") + + +def test_mentions_version_matches_whole_versions_only(): + assert check.mentions_version("now 2.0.0.", "2.0.0") + assert not check.mentions_version("now 12.0.0", "2.0.0") + assert not check.mentions_version("now 2.0.01", "2.0.0") + + +# --- the version bump the newest release needs ------------------------------- + + +def _releases(newest: str, previous: str, body: str) -> str: + return ( + f"## [Unreleased]\n\n## [{newest}] - 2026-08-01\n\n{body}\n" + f"## [{previous}] - 2026-07-01\n\n### Added\n\n- old\n" + ) + + +@pytest.mark.parametrize( + "newest, previous, body, needed", + [ + ("0.4.1", "0.4.0", "### Fixed\n\n- a bug\n", None), + ("0.4.1", "0.4.0", "### Security\n\n- a fix\n", None), + ("0.4.1", "0.4.0", "### Removed\n\n- `x`\n", "a minor release (0.5.0)"), + ("0.4.1", "0.4.0", "### Deprecated\n\n- `x`\n", "a minor release (0.5.0)"), + ("0.4.1", "0.4.0", "### Changed\n\n- **Breaking:** y\n", "(0.5.0)"), + ("0.5.0", "0.4.0", "### Removed\n\n- `x`\n", None), # pre-1.0: minor + ("1.0.0", "0.4.0", "### Removed\n\n- `x`\n", None), + ("1.2.1", "1.2.0", "### Deprecated\n\n- `x`\n", "a minor release (1.3.0)"), + ("1.3.0", "1.2.0", "### Deprecated\n\n- `x`\n", None), + ("1.3.0", "1.2.0", "### Removed\n\n- `x`\n", "a major release (2.0.0)"), + ("1.2.1", "1.2.0", "### Removed\n\n- `x`\n", "a major release (2.0.0)"), + ("2.0.0", "1.2.0", "### Removed\n\n- `x`\n", None), + ], +) +def test_release_bump_rule(newest, previous, body, needed): + errors = check.release_bump_errors( + check.parse_changelog(_releases(newest, previous, body)) + ) + if needed is None: + assert errors == [] + else: + assert len(errors) == 1 + assert needed in errors[0] + assert "docs/lifecycle.md#the-package" in errors[0] + + +def test_release_bump_rule_needs_two_dated_releases(): + text = "## [Unreleased]\n\n## [0.4.1] - 2026-08-01\n\n### Removed\n\n- `x`\n" + assert check.release_bump_errors(check.parse_changelog(text)) == [] + + +# --- a synthetic repository ----------------------------------------------------- + + +def _git(repo, *args): + subprocess.run( + ["git", "-C", str(repo), *args], + check=True, + capture_output=True, + env={ + **os.environ, + "GIT_AUTHOR_NAME": "t", + "GIT_AUTHOR_EMAIL": "t@example.com", + "GIT_COMMITTER_NAME": "t", + "GIT_COMMITTER_EMAIL": "t@example.com", + }, + ) + + +def _write(path: Path, text: str) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_bytes(text.encode("utf-8")) # LF on every platform + + +def write_skill(repo, name, version="1.2.0", agents=("claude", "codex"), **deprec): + """Write skill ``name``; ``deprec`` (since, reason, replacement) deprecates.""" + lines = [ + f"name: {name}", + f"description: The {name} skill.", + "category: testing", + f"version: {version}", + "supported-agents:", + *(f" - {agent}" for agent in agents), + ] + if deprec: + lines += ["deprecated:", *(f" {k}: {v}" for k, v in deprec.items())] + _write(repo / SKILLS / name / "meta.yaml", "\n".join(lines) + "\n") + _write(repo / SKILLS / name / "skill.md", f"# {name}\n") + + +def write_changelog(repo, unreleased="", released=""): + _write( + repo / "CHANGELOG.md", + "# Changelog\n\n## [Unreleased]\n\n" + f"{unreleased}\n{released}" + "## [0.1.0] - 2026-01-01\n\n### Added\n\n- The first release.\n", + ) + + +def write_schema(repo, version=1): + schema = {"properties": {"schema_version": {"const": version}}} + _write(repo / check.CATALOG_SCHEMA_PATH, json.dumps(schema)) + + +def write_version(repo, version): + _write(repo / "pyproject.toml", f'[project]\nname = "x"\nversion = "{version}"\n') + + +def commit(repo, message="change"): + _git(repo, "add", "-A") + _git(repo, "commit", "-q", "-m", message) + + +@pytest.fixture +def repo(tmp_path, monkeypatch): + """``main``: two skills and every current adapter, released as nothing yet.""" + root = tmp_path / "repo" + root.mkdir() + _git(root, "init", "-q", "-b", "main") + write_skill(root, "old-review") + write_skill(root, "other-review", agents=("claude", "codex", "copilot")) + contracts = {"adapters": {name: {} for name in sorted(ALL_ADAPTERS)}} + _write(root / check.CONTRACTS_PATH, json.dumps(contracts)) + write_schema(root) + write_changelog(root) + write_version(root, "0.1.0") + commit(root, "base") + monkeypatch.setattr(check, "ROOT", root) + return root + + +def errors(today=TODAY): + return check.lifecycle_errors("main", today) + + +def test_no_compatibility_change_needs_no_note(repo): + write_skill(repo, "old-review", version="1.2.1") # a patch: no note + write_skill(repo, "brand-new") # additions need no lifecycle note + assert errors() == [] + + +def test_the_base_ref_must_exist(repo): + assert check.lifecycle_errors("origin/nope") == [ + "base ref 'origin/nope' not found (fetch it first)" + ] + + +def test_base_needs_a_git_checkout(tmp_path, monkeypatch): + monkeypatch.setattr(check, "ROOT", tmp_path) + monkeypatch.setenv("GIT_CEILING_DIRECTORIES", str(tmp_path.parent)) + assert check.lifecycle_errors("main") == ["--base main needs a git checkout"] + + +# --- removing a skill ----------------------------------------------------------- + + +def test_removing_a_skill_needs_a_removed_entry_and_a_deprecation(repo): + shutil.rmtree(repo / SKILLS / "old-review") + found = errors() + assert len(found) == 2 + assert found[0].startswith( + "skill `old-review` was removed: add a `### Removed` entry naming " + "`old-review` (in backticks) under `## [Unreleased]` in CHANGELOG.md" + ) + assert "docs/lifecycle.md#removing-a-skill" in found[0] + assert "without being deprecated first" in found[1] + assert "`### Security` entry naming `old-review`" in found[1] + + write_changelog(repo, "### Removed\n\n- `old-review`: use `other-review`.\n") + assert [e for e in errors() if "deprecated first" not in e] == [] + + +def test_an_entry_in_the_wrong_group_or_section_does_not_count(repo): + shutil.rmtree(repo / SKILLS / "old-review") + write_changelog( + repo, + "### Changed\n\n- `old-review` is gone.\n", + "## [0.2.0] - 2026-02-01\n\n### Added\n\n- `old-review`\n\n" + "## [0.1.1] - 2026-01-15\n\n### Removed\n\n- `old-review`\n\n", + ) + assert any("add a `### Removed` entry" in e for e in errors()) + + +def test_an_urgent_security_removal_skips_the_deprecation(repo): + shutil.rmtree(repo / SKILLS / "old-review") + write_changelog( + repo, + "### Security\n\n- `old-review` told agents to disable TLS checks.\n\n" + "### Removed\n\n- `old-review`, for the reason under Security.\n", + ) + assert errors() == [] + + +def test_removing_a_deprecated_skill_no_release_contains(repo): + # never in a tagged release: nobody holds a release to give notice to + write_skill(repo, "old-review", reason="Obsolete.", since="1.2.0") + write_changelog(repo, "### Deprecated\n\n- `old-review`\n") + commit(repo) + shutil.rmtree(repo / SKILLS / "old-review") + write_changelog(repo, "### Removed\n\n- `old-review`\n") + assert errors() == [] + + +@pytest.fixture +def released_deprecation(repo): + """``old-review`` shipped deprecated in the tagged release 0.2.0.""" + write_skill(repo, "old-review", reason="Obsolete.", since="1.2.0") + deprecated = ( + "## [0.2.0] - 2026-06-01\n\n### Deprecated\n\n" + "- `old-review`: obsolete; removal no earlier than 90 days from now.\n\n" + ) + write_changelog(repo, released=deprecated) + commit(repo, "release 0.2.0") + _git(repo, "tag", "v0.2.0") + shutil.rmtree(repo / SKILLS / "old-review") + write_changelog(repo, "### Removed\n\n- `old-review`\n", deprecated) + return repo + + +def test_removal_waits_for_the_notice_period(released_deprecation): + assert errors(datetime.date(2026, 8, 29)) == [ + "skill `old-review` was removed too early: its deprecation was published " + "in 0.2.0 on 2026-06-01, so it can be removed from 2026-08-30 (90 days' " + "notice; docs/lifecycle.md#removing-a-skill)" + ] + assert errors(datetime.date(2026, 8, 30)) == [] + + +def test_from_1_0_the_notice_period_is_180_days(released_deprecation): + write_version(released_deprecation, "1.0.0") + (found,) = errors(datetime.date(2026, 11, 27)) + assert "can be removed from 2026-11-28 (180 days' notice" in found + assert errors(datetime.date(2026, 11, 28)) == [] + + +def test_the_notice_counts_only_from_a_published_release(released_deprecation): + # a dated section without its tag is prepared, not published + _git(released_deprecation, "tag", "-d", "v0.2.0") + _git(released_deprecation, "tag", "v0.1.0", "HEAD") # the skill was released + (found,) = errors(datetime.date(2027, 1, 1)) + assert "no published release announces its deprecation" in found + + +# --- deprecating, dropping an agent, a major version, the catalog schema ------ + + +def test_a_new_deprecation_needs_a_deprecated_entry(repo): + write_skill(repo, "old-review", "1.3.0", since="1.3.0", reason="Obsolete.") + assert errors() == [ + "skill `old-review` is newly deprecated: add a `### Deprecated` entry " + "naming `old-review` (in backticks) under `## [Unreleased]` in " + "CHANGELOG.md, naming its replacement or the reason " + "(docs/lifecycle.md#deprecating-a-skill)" + ] + write_changelog(repo, "### Deprecated\n\n- `old-review`: obsolete.\n") + assert errors() == [] + + +def test_the_newest_dated_section_counts_after_a_release_cut(repo): + write_skill(repo, "old-review", "1.3.0", since="1.3.0", reason="Obsolete.") + write_changelog( + repo, released="## [0.2.0] - 2026-09-01\n\n### Deprecated\n\n- `old-review`\n" + ) + assert errors() == [] + + +def test_a_major_skill_version_needs_a_changed_entry(repo): + write_skill(repo, "old-review", version="2.0.0") + (found,) = errors() + assert found.startswith( + "skill `old-review` went from 1.2.0 to 2.0.0, a major (breaking) change: " + "add a `### Changed` entry naming `old-review` and 2.0.0" + ) + write_changelog(repo, "### Changed\n\n- `old-review` now reports X.\n") + assert errors() == [found] # the entry must give the new version + write_changelog(repo, "### Changed\n\n- `old-review` 2.0.0 drops area Y.\n") + assert errors() == [] + + +def test_a_catalog_schema_version_change_needs_a_breaking_entry(repo): + write_schema(repo, 2) + (found,) = errors() + assert "catalog schema_version changed from 1 to 2" in found + write_changelog( + repo, "### Changed\n\n- **Breaking:** the catalog `schema_version` is 2.\n" + ) + assert errors() == [] + + +# --- walkthrough (a): renaming a skill ------------------------------------------ + + +def _catalog(repo): + skills = discover_skills(repo / SKILLS, known_agents=ADAPTERS) + catalog = build_catalog(skills, content_manifest(__version__, skills)) + assert schema_errors(catalog, exact=True) == [] + return {skill["name"]: skill["deprecated"] for skill in catalog["skills"]} + + +def test_walkthrough_renaming_a_skill(repo): + """old-review becomes new-review: add, deprecate, wait, remove.""" + _git(repo, "tag", "v0.1.0") # old-review is in a release + + # 1. the rename PR: the new name, and the old one deprecated in its favour + write_skill(repo, "new-review", version="1.3.0") + write_skill( + repo, + "old-review", + version="1.3.0", + since="1.3.0", + replacement="new-review", + reason="Renamed to new-review.", + ) + assert _catalog(repo) == { + "new-review": None, + "old-review": { + "since": "1.3.0", + "replacement": "new-review", + "reason": "Renamed to new-review.", + }, + "other-review": None, + } + assert [e.split(":")[0] for e in errors()] == [ + "skill `old-review` is newly deprecated" + ] + notes = ( + "### Added\n\n- `new-review` (1.3.0), the new name of `old-review`.\n\n" + "### Deprecated\n\n- `old-review`: renamed to `new-review`; install that " + "instead. It will be removed no earlier than 90 days after this release.\n" + ) + write_changelog(repo, notes) + assert errors() == [] + + # 2. the release that publishes the deprecation + released = notes.replace("### Added", "## [0.2.0] - 2026-06-01\n\n### Added") + write_changelog(repo, released=released + "\n") + commit(repo, "release 0.2.0") + _git(repo, "tag", "v0.2.0") + + # 3. at least 90 days later, the removal PR + shutil.rmtree(repo / SKILLS / "old-review") + assert [e.split(":")[0] for e in errors(datetime.date(2026, 9, 1))] == [ + "skill `old-review` was removed" + ] + write_changelog( + repo, + "### Removed\n\n- `old-review`, deprecated in 0.2.0: use `new-review`.\n", + released + "\n", + ) + assert errors(datetime.date(2026, 8, 29)) != [] # too early + assert errors(datetime.date(2026, 9, 1)) == [] + # a removed skill is simply absent: the CHANGELOG records it + assert _catalog(repo) == {"new-review": None, "other-review": None} + + +def test_the_catalog_represents_every_lifecycle_state(repo): + write_skill(repo, "new-review") + write_skill(repo, "old-review", since="1.2.0", replacement="new-review", reason="R") + write_skill(repo, "other-review", since="1.2.0", reason="No longer maintained.") + assert _catalog(repo) == { + "new-review": None, # active + "old-review": {"since": "1.2.0", "replacement": "new-review", "reason": "R"}, + "other-review": { # deprecated with a rationale only + "since": "1.2.0", + "replacement": None, + "reason": "No longer maintained.", + }, + } + + +# --- walkthrough (b): removing an agent ----------------------------------------- + + +def test_walkthrough_dropping_an_agent_from_a_skill(repo): + write_skill(repo, "other-review", agents=("claude", "codex")) + (found,) = errors() + assert found.startswith( + "skill `other-review` no longer supports `copilot`: add one `### Removed` " + "entry naming both `other-review` and `copilot` (in backticks)" + ) + assert "`skilldeck uninstall other-review --agent copilot`" in found + # naming them in two different entries is not enough + write_changelog(repo, "### Removed\n\n- `other-review`\n- `copilot` things\n") + assert errors() == [found] + write_changelog( + repo, + "### Removed\n\n- `other-review` no longer supports `copilot`: remove " + "installed copies with `skilldeck uninstall other-review --agent copilot`.\n", + ) + assert errors() == [] + + +@pytest.mark.parametrize("adapter", ["copilot", "kiro-steering"]) +def test_walkthrough_removing_an_adapter(repo, monkeypatch, adapter): + remaining = {k: v for k, v in ALL_ADAPTERS.items() if k != adapter} + monkeypatch.setattr(check, "ALL_ADAPTERS", remaining) + if adapter in ADAPTERS: + # skills can no longer list a removed native agent; that is covered by + # the adapter's note, not one per skill + write_skill(repo, "other-review", agents=("claude", "codex")) + assert errors() == [ + f"adapter `{adapter}` was removed: add a `### Removed` entry naming " + f"`{adapter}` (in backticks) under `## [Unreleased]` in CHANGELOG.md, " + "listing the paths it installed to so users can delete what is left " + "(docs/lifecycle.md#removing-an-agent-or-format)" + ] + write_changelog(repo, f"### Removed\n\n- The `{adapter}` adapter.\n") + assert errors() == [] + + +def test_main_reports_what_is_missing_and_links_the_policy(repo, capsys): + shutil.rmtree(repo / SKILLS / "old-review") + assert check.main(["--base", "main"]) == 1 + err = capsys.readouterr().err + assert err.startswith("error: skill `old-review` was removed") + assert "See docs/lifecycle.md" in err + + +# --- what happens to installed copies (docs/lifecycle.md says so) ------------- + + +def _runner(): + # Click < 8.2 mixes stderr into stdout unless told not to; 8.2 dropped the + # flag and always captures the streams separately + try: + return CliRunner(mix_stderr=False) # type: ignore[call-arg] + except TypeError: + return CliRunner() + + +def _run(*args): + result = _runner().invoke(cli, list(args)) + assert result.exit_code == 0, (result.stdout, result.stderr) + return result + + +@pytest.fixture +def installed(repo, tmp_path, monkeypatch): + """Both skills installed for claude and codex in a project.""" + monkeypatch.setattr(registry, "DEFAULT_SKILLS_DIR", repo / SKILLS) + project = tmp_path / "project" + project.mkdir() + monkeypatch.chdir(project) + _run("install", "--all", "--agent", "claude", "--agent", "codex") + return project + + +def test_installed_copies_of_a_removed_skill(repo, installed): + shutil.rmtree(repo / SKILLS / "old-review") + left = Path(".claude/skills/old-review/SKILL.md") + status = _run("status", "--agent", "claude").stdout + assert f"orphan: {Path.cwd().resolve() / left} (old-review 1.2.0)" in status + assert "nothing to update" in _run("update", "--agent", "claude").stdout + result = _run("uninstall", "--all", "--agent", "claude") + assert "old-review" not in result.stdout + assert left.is_file() # skilldeck leaves it: delete it by hand + with pytest.raises(registry.SkillError, match="unknown skill: old-review"): + _runner().invoke( + cli, + ["uninstall", "old-review", "--agent", "claude"], + catch_exceptions=False, + ) + + +def test_installed_copies_for_an_agent_a_skill_dropped(repo, installed): + write_skill(repo, "other-review", version="1.3.0", agents=("claude",)) + kept = Path(".agents/skills/other-review/SKILL.md") + # status and update for that agent no longer look at it... + assert "other-review" not in _run("status", "--agent", "codex").stdout + assert "other-review" not in _run("update", "--agent", "codex").stdout + assert "1.2.0" in kept.read_text(encoding="utf-8") + # ...but uninstall still removes it + removed = _run("uninstall", "other-review", "--agent", "codex").stdout + assert f"removed other-review <- {Path.cwd().resolve() / kept}" in removed + assert not kept.exists() + + +# --- walkthrough (c): a stamp-format migration ----------------------------------- + +real_stamp = adapter_base.stamp +real_parse = adapter_base.parse + +# A hypothetical second stamp format, with the explicit marker that +# docs/lifecycle.md#install-stamps asks the next format to carry. +_V2_RE = re.compile( + r"\n?" +) + + +def _stamp_v2(content, name, version): + v1 = real_stamp(content, name, version) + return v1.replace("\n" +) + + +def test_the_current_stamp_format_is_pinned(): + assert stamp("BODY\n", "demo", "1.2.3") == V1_STAMPED + assert parse(V1_STAMPED) == Stamp(name="demo", version="1.2.3", modified=False) + + +def test_a_stamp_in_an_unknown_future_format_reads_as_unstamped(): + # what this version does with a file a newer skilldeck stamped in another + # format (see docs/lifecycle.md#install-stamps): it is not a skilldeck + # file here, so install, update and uninstall leave it alone without --force + future = V1_STAMPED.replace("skilldeck name=", "skilldeck stamp=2 name=") + assert parse(future) is None From 73215112cc151a56a0a75ec20145e2b74278a0f0 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 25 Sep 2026 03:26:35 +0000 Subject: [PATCH 2/2] Tighten the lifecycle check and policy after review CHANGELOG: restore the Added bullets that #53 left under Security in [Unreleased], and file the internal get_skill removal under Changed, so Removed means public-surface removals only. check_lifecycle.py: - the urgent security path now needs a Removed entry marked **Security:** plus a Security entry naming the skill; a passing mention no longer exempts a removal; - only sections the PR adds count: [Unreleased], or a dated section the base's CHANGELOG lacks; a published section no longer does; - the notice period comes from release tags reachable from the base: the tag's own meta.yaml and CHANGELOG must deprecate the skill, counted from the later of the section date and the tag date, so it can't be retrofitted or back-dated; - an adapter removal needs a Removed entry of its own naming no skill; - with no release tags it prints a note and skips the notice rules; - an impossible CHANGELOG date is a clean error, here and in prepare_release.py. Policy: - skills keep SemVer's 0.x exception; - new features ship in minor releases; - the 90/180-day notice periods and the 7-day security target are marked as maintainer choices, and SECURITY.md states the target; - eval run records follow their own schema_version; - the lockfile section is trimmed to rules. Tests reuse the reviewer's probes A-E and G as regressions. The catalog schema validator moves to tests/_schema.py. Closes #78 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01HtiCGzpikMrkDYBkfQG5CX --- .github/workflows/ci.yml | 3 +- CHANGELOG.md | 215 +++++++++++----------- CLAUDE.md | 38 ++-- CONTRIBUTING.md | 3 +- SECURITY.md | 4 +- docs/authoring-skills.md | 3 +- docs/lifecycle.md | 178 +++++++++++------- docs/releasing.md | 16 +- scripts/check_lifecycle.py | 329 +++++++++++++++++++++++----------- scripts/prepare_release.py | 6 +- tests/_schema.py | 121 +++++++++++++ tests/test_catalog.py | 122 +------------ tests/test_lifecycle.py | 231 ++++++++++++++++++++---- tests/test_prepare_release.py | 13 ++ 14 files changed, 828 insertions(+), 454 deletions(-) create mode 100644 tests/_schema.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5050a24..7528c0f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,7 +19,8 @@ jobs: with: persist-credentials: false # all branches and tags: the plugin release record is checked - # against the PR's base branch and any release tag + # against the PR's base branch and any release tag, and the + # lifecycle check reads the release tags fetch-depth: 0 - uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d # v10.0.1 with: diff --git a/CHANGELOG.md b/CHANGELOG.md index c33fe9b..15dd64e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,88 +20,6 @@ All notable changes to this project are documented here. The format is based on updates to that SHA-pinned action carry it. A test rejects inline tool pins in workflows and keeps the documented pypi-attestations command in step with the pin. - -- `authentication-review` skill (0.1.0) — reviews authentication changes in - depth: password storage and policy, recovery/reset flows, MFA bypass and OTP - handling, session fixation and cookie hardening, JWT/API-token verification - (alg confusion, key selection), OAuth 2.0 (PKCE, `state`, `redirect_uri` - exact match, deprecated grants), OIDC (`nonce`, `iss`+`sub` identity, JWKS - trust), SAML (assertion vs response signatures, XSW, replay/conditions), and - LDAP sign-in (empty-password bind, unchecked bind result). Classified - against OWASP ASVS 5.0 (V6/V7/V9/V10, with V11/V3 for KDF and cookie - findings) with patterns from RFC 9700, NIST SP 800-63B, and the OWASP - Authentication, Password Storage, Session Management, MFA, Forgot Password, - and SAML Security cheat sheets. Pairs with `security-review` (bumped to - 0.3.2 for the reciprocal cross-reference), which keeps breadth coverage. - Ships with OAuth (hand-rolled code flow missing `state`/PKCE) and SAML - (response-envelope-only signature check + raw-document `NameID` read) eval - fixtures. -- `ci-workflow-review` skill (0.2.0) — reviews CI/CD pipeline changes for - injection and poisoned pipeline execution (untrusted `github.event` / - GitLab predefined-variable interpolation, `pull_request_target` + head - checkout, fork MR pipelines), credential and token scope - (`GITHUB_TOKEN` permissions, `CI_JOB_TOKEN` allowlist, masked/protected - variables), unpinned third-party steps and `include:`s, artifact/cache - integrity, and runner exposure (self-hosted runners, GitLab privileged - Docker/DinD and shell executors). Classified against the OWASP Top 10 CI/CD - Security Risks (CICD-SEC-1–10) with patterns from GitHub's Actions hardening - guide and GitLab's pipeline/job-token/runner security guidance. Ships with - GitHub and GitLab eval fixtures (`workflow_run` artifact poisoning through - `$GITHUB_ENV` plus a tag-pinned third-party action; a privileged DinD runner - plus MR-title injection in a fork-reachable job). -- `iac-review` skill (0.1.0) — reviews infrastructure-as-code changes - (Terraform, CloudFormation, Kubernetes/Helm, Dockerfiles) for network - exposure, wildcard IAM, secrets in code/state, missing encryption, container - hardening per the Kubernetes Pod Security Standards and the OWASP Docker - cheat sheet, and stateful-resource change safety; anchored to CIS benchmark - baselines. Ships with an eval fixture (wildcard S3 policy on an app role). -- Golden-diff eval harness (`evals/`) (#32): seven fixtures — one per skill — - each a tiny repo whose diff contains a planted defect (path traversal, - one-step column rename, assertion-free test, retry without backoff on a - non-idempotent POST, log injection, dependency confusion, duplicate code). - `python evals/run_evals.py` builds each repo, - installs the skill, invokes an agent (default: Claude CLI), and scores the - report: plants must be found and total findings must stay under a cap. Runs - manually (paid API); CI validates fixture structure only. -- `scripts/prepare_release.py ` automates release prep: bumps - `pyproject.toml`, dates the `[Unreleased]` CHANGELOG section, re-locks, - regenerates the plugin tree, and re-runs the consistency guard - (`docs/releasing.md` updated to make it the documented path) (#35). -- The repo is now a Claude Code plugin marketplace (#31): - `/plugin marketplace add IcebergAI/skilldeck` then - `/plugin install skilldeck@skilldeck` installs all skills with no Python - tooling. The committed plugin tree (`.claude-plugin/marketplace.json` + - `claude-plugin/`) is generated from the canonical skills by - `scripts/build_plugin.py`; a pytest freshness guard fails if it drifts. -- Cursor and GitHub Copilot adapters (#30). Cursor installs agent-requested - rules to `.cursor/rules/.mdc` (`description` + `alwaysApply: false`); - Copilot installs prompt files to `.github/prompts/.prompt.md`, run - with `/` in chat. Both are project-scope only — neither tool has a - stable filesystem location for user-level config — enforced by a new - per-adapter `scopes` attribute. All skills add the two agents to - `supported-agents` (patch version bumps). These formats are now the - `cursor-rule` and `copilot-prompt` legacy adapters; `cursor` and `copilot` - install `SKILL.md` folders at both scopes (see Changed, #100). -- `install`/`uninstall` accept `--agent` multiple times, or `--agent all`, to - target several agents in one command; `skilldeck show ` prints a - skill's body (or, with `--agent`, the rendered per-agent output) before - installing (#29). -- Installed skills are now stamped with a `skilldeck` comment recording the - skill name, version, and a content hash. New commands build on it: - `skilldeck status --agent ` shows installed vs bundled versions - (up to date / stale / modified locally / unmanaged, plus orphans of skills no - longer bundled) and `skilldeck update --agent ` refreshes stale installs - (#27). -- `install` no longer silently overwrites: a destination file with local - modifications — or one skilldeck didn't write — is refused unless `--force` - is given; `update` likewise skips modified installs without `--force` (#28). - Note: installs made by skilldeck ≤ 0.3.0 carry no stamp, so the first - reinstall over them needs `--force` once, and so does uninstalling them - (#95). -- Structural lint tests (`tests/test_skill_structure.py`) asserting every - bundled skill body carries the standardized elements: a Scope section with - the uncommitted-changes fallback, severity anchors, a worked example, the - verify-before-reporting instruction, and the one-line report header (#33). - The release workflow now gates publication (#108): a `verify` job fails unless the tagged commit is reachable from `main` and runs lint, type-check, and the test suite on it before anything is built. That catches a tag pushed @@ -353,6 +271,8 @@ All notable changes to this project are documented here. The format is based on the `pull_request_target` head checkout) plus a third-party action pinned by tag. Their `expected.yaml` keywords, severity floors and sample reports are updated to match. +- Dropped the dead internal `skilldeck.registry.get_skill` helper (unused, + and it skipped `supported-agents` validation). ### Fixed @@ -509,11 +429,6 @@ All notable changes to this project are documented here. The format is based on version bump and a release PR whose plugin drifted to a development version after `prepare_release.py`. -### Removed - -- Dead `skilldeck.registry.get_skill` helper (unused, and it skipped - `supported-agents` validation). - ### Added - Lifecycle and compatibility policy, `docs/lifecycle.md` (#78), linked from @@ -522,35 +437,44 @@ All notable changes to this project are documented here. The format is based on It defines: - what bumps the package version before and after 1.0 (a patch release - never removes, deprecates or breaks anything); - - what makes a skill change major, minor or patch (skills don't get SemVer's - 0.x exception), and how skill versions meet `status` and `update`; + never removes, deprecates, adds or breaks anything), and that + `### Removed` entries are only for removals from the public surface; + - what makes a skill change major, minor or patch (while a skill is 0.x, a + breaking change bumps its minor version), and how skill versions meet + `status` and `update`; - the deprecate, notice and remove path for skills, agents and formats: a deprecation must ship in a tagged release at least 90 days before the - removal (180 days from 1.0). A rename is a new skill plus a deprecation; + removal (180 days from 1.0; both are maintainer policy choices). A + rename is a new skill plus a deprecation; - what `status`, `update` and `uninstall` do with installed copies of a removed skill, or of a skill that dropped an agent; - how `meta.yaml`, the catalog, install stamps and future lockfiles (#71) may change. Every stamp format stays readable, and `update` migrates old stamps; - - which output is a stable contract (`catalog --json`, `provenance --json`, - run records) and which is human output that may change; - - the urgent security-fix path. + - which output is a stable contract (`catalog --json`, `provenance --json`) + and which is human output that may change; eval run records follow their + own schema version; + - the urgent security-fix path, with a 7-day release target for confirmed + high- and critical-severity issues (also added to `SECURITY.md`). Tests walk through a skill rename, an agent removal and a stamp-format migration, and pin the current stamp format byte for byte. - `scripts/check_lifecycle.py`, run by CI's `lint` job with `--base origin/`, requires CHANGELOG notes when compatibility - changes. Each note is a bullet under `[Unreleased]` (or the newest dated - section) that names the skill, agent or adapter in backticks: - - - a removed skill needs a Removed entry, a deprecation at the base, and, once - a release has contained the skill, a published Deprecated entry at least - the notice period old. A Security entry naming the skill skips the last - two; + changes. Each note is a bullet in a section the pull request adds + (`[Unreleased]`, or a release it cuts) that names the skill, agent or + adapter in backticks: + + - a removed skill needs a Removed entry and a deprecation at the base. Once + a release has contained the skill, a release tag must also have shipped + the deprecation (in its own `meta.yaml` and CHANGELOG) at least the + notice period before, counted from the later of the section date and the + tag date. A Removed entry marked as a security removal, together with a + Security entry naming the skill, skips the last two; - a newly deprecated skill needs a Deprecated entry; - - an agent dropped from a skill, or a removed adapter, needs a Removed entry; - - a major skill version needs a Changed entry giving the new version; + - an agent dropped from a skill needs a Removed entry naming both, and a + removed adapter needs a Removed entry of its own that names no skill; + - a new major skill version needs a Changed entry giving the new version; - a change to the catalog's schema version needs an entry marked as breaking. @@ -558,7 +482,9 @@ All notable changes to this project are documented here. The format is based on anything, when the newest dated section is a patch release with Removed, Deprecated or breaking entries (from 1.0, Removed and breaking entries need a major release). Every error says what to add and where, and links the - policy. + policy. Without release tags it prints a note and skips the notice rules. + An impossible CHANGELOG date is a clean error here and in + `prepare_release.py`. - Agent compatibility matrix, `docs/compatibility.md` (#79), linked from the README and `docs/adapters.md`. It lists every adapter, including the `copilot-prompt`, `cursor-rule` and `kiro-steering` legacy adapters, with: @@ -749,6 +675,87 @@ All notable changes to this project are documented here. The format is based on Plain `provenance` reports only the identities embedded at build time; the docs now say so. CI and the release workflow run the installed wheel and sdist with `--verify` (#109). +- `authentication-review` skill (0.1.0) — reviews authentication changes in + depth: password storage and policy, recovery/reset flows, MFA bypass and OTP + handling, session fixation and cookie hardening, JWT/API-token verification + (alg confusion, key selection), OAuth 2.0 (PKCE, `state`, `redirect_uri` + exact match, deprecated grants), OIDC (`nonce`, `iss`+`sub` identity, JWKS + trust), SAML (assertion vs response signatures, XSW, replay/conditions), and + LDAP sign-in (empty-password bind, unchecked bind result). Classified + against OWASP ASVS 5.0 (V6/V7/V9/V10, with V11/V3 for KDF and cookie + findings) with patterns from RFC 9700, NIST SP 800-63B, and the OWASP + Authentication, Password Storage, Session Management, MFA, Forgot Password, + and SAML Security cheat sheets. Pairs with `security-review` (bumped to + 0.3.2 for the reciprocal cross-reference), which keeps breadth coverage. + Ships with OAuth (hand-rolled code flow missing `state`/PKCE) and SAML + (response-envelope-only signature check + raw-document `NameID` read) eval + fixtures. +- `ci-workflow-review` skill (0.2.0) — reviews CI/CD pipeline changes for + injection and poisoned pipeline execution (untrusted `github.event` / + GitLab predefined-variable interpolation, `pull_request_target` + head + checkout, fork MR pipelines), credential and token scope + (`GITHUB_TOKEN` permissions, `CI_JOB_TOKEN` allowlist, masked/protected + variables), unpinned third-party steps and `include:`s, artifact/cache + integrity, and runner exposure (self-hosted runners, GitLab privileged + Docker/DinD and shell executors). Classified against the OWASP Top 10 CI/CD + Security Risks (CICD-SEC-1–10) with patterns from GitHub's Actions hardening + guide and GitLab's pipeline/job-token/runner security guidance. Ships with + GitHub and GitLab eval fixtures (`workflow_run` artifact poisoning through + `$GITHUB_ENV` plus a tag-pinned third-party action; a privileged DinD runner + plus MR-title injection in a fork-reachable job). +- `iac-review` skill (0.1.0) — reviews infrastructure-as-code changes + (Terraform, CloudFormation, Kubernetes/Helm, Dockerfiles) for network + exposure, wildcard IAM, secrets in code/state, missing encryption, container + hardening per the Kubernetes Pod Security Standards and the OWASP Docker + cheat sheet, and stateful-resource change safety; anchored to CIS benchmark + baselines. Ships with an eval fixture (wildcard S3 policy on an app role). +- Golden-diff eval harness (`evals/`) (#32): seven fixtures — one per skill — + each a tiny repo whose diff contains a planted defect (path traversal, + one-step column rename, assertion-free test, retry without backoff on a + non-idempotent POST, log injection, dependency confusion, duplicate code). + `python evals/run_evals.py` builds each repo, + installs the skill, invokes an agent (default: Claude CLI), and scores the + report: plants must be found and total findings must stay under a cap. Runs + manually (paid API); CI validates fixture structure only. +- `scripts/prepare_release.py ` automates release prep: bumps + `pyproject.toml`, dates the `[Unreleased]` CHANGELOG section, re-locks, + regenerates the plugin tree, and re-runs the consistency guard + (`docs/releasing.md` updated to make it the documented path) (#35). +- The repo is now a Claude Code plugin marketplace (#31): + `/plugin marketplace add IcebergAI/skilldeck` then + `/plugin install skilldeck@skilldeck` installs all skills with no Python + tooling. The committed plugin tree (`.claude-plugin/marketplace.json` + + `claude-plugin/`) is generated from the canonical skills by + `scripts/build_plugin.py`; a pytest freshness guard fails if it drifts. +- Cursor and GitHub Copilot adapters (#30). Cursor installs agent-requested + rules to `.cursor/rules/.mdc` (`description` + `alwaysApply: false`); + Copilot installs prompt files to `.github/prompts/.prompt.md`, run + with `/` in chat. Both are project-scope only — neither tool has a + stable filesystem location for user-level config — enforced by a new + per-adapter `scopes` attribute. All skills add the two agents to + `supported-agents` (patch version bumps). These formats are now the + `cursor-rule` and `copilot-prompt` legacy adapters; `cursor` and `copilot` + install `SKILL.md` folders at both scopes (see Changed, #100). +- `install`/`uninstall` accept `--agent` multiple times, or `--agent all`, to + target several agents in one command; `skilldeck show ` prints a + skill's body (or, with `--agent`, the rendered per-agent output) before + installing (#29). +- Installed skills are now stamped with a `skilldeck` comment recording the + skill name, version, and a content hash. New commands build on it: + `skilldeck status --agent ` shows installed vs bundled versions + (up to date / stale / modified locally / unmanaged, plus orphans of skills no + longer bundled) and `skilldeck update --agent ` refreshes stale installs + (#27). +- `install` no longer silently overwrites: a destination file with local + modifications — or one skilldeck didn't write — is refused unless `--force` + is given; `update` likewise skips modified installs without `--force` (#28). + Note: installs made by skilldeck ≤ 0.3.0 carry no stamp, so the first + reinstall over them needs `--force` once, and so does uninstalling them + (#95). +- Structural lint tests (`tests/test_skill_structure.py`) asserting every + bundled skill body carries the standardized elements: a Scope section with + the uncommitted-changes fallback, severity anchors, a worked example, the + verify-before-reporting instruction, and the one-line report header (#33). ## [0.3.0] - 2026-06-27 diff --git a/CLAUDE.md b/CLAUDE.md index 8037014..30bd2f6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -142,21 +142,29 @@ by `--agent all`); `skilldeck migrate` moves old-format installs to `SKILL.md`. `pytest`. A dated CHANGELOG section without a matching `v*` tag is prepared, not published. - Lifecycle: follow `docs/lifecycle.md` for what bumps package/skill versions - (skills: removing a checklist area or changing output shape is major, adding - checks minor, wording/citations patch) and for deprecation → notice → removal - (a deprecation must ship in a tagged release ≥90 days before removal, 180 - from 1.0; urgent security fixes may skip it). `scripts/check_lifecycle.py` - (CI `lint` job, `--base origin/`; needs `uv run --locked --extra dev`) - fails a PR unless CHANGELOG `[Unreleased]` (or the newest dated section) has a - bullet naming the thing **in backticks**: `### Removed` for a removed skill - (which must also be deprecated at base with a published, old-enough - `### Deprecated` entry, unless a `### Security` entry names it), a removed - adapter, or an agent dropped from a skill (skill and agent in one bullet); - `### Deprecated` for a newly deprecated skill; `### Changed` naming skill + - new version for a major skill bump; `**Breaking:**` + `schema_version` for a - catalog schema bump. It also fails (as does `prepare_release.py`) when the - newest dated section is a patch release with Removed/Deprecated/**Breaking** - entries. Run it locally with `--base origin/main` before pushing. + (skills from 1.0.0: removing a checklist area or changing output shape is + major, adding checks minor, wording/citations patch; while a skill is 0.x a + breaking change bumps its minor) and for deprecation → notice → removal (a + release tag must ship the deprecation ≥90 days before removal, 180 from 1.0). + `### Removed` is only for public-surface removals (skills, agents/adapters, + commands/options, stable output or `meta.yaml` fields); internal code + removals go under Changed. `scripts/check_lifecycle.py` (CI `lint` job, + `--base origin/`; needs `uv run --locked --extra dev`) fails a PR + unless a section the PR adds (`[Unreleased]`, or a release it cuts) has a + bullet naming the thing **in its own backticks**: `### Removed` for a removed + skill (also deprecated at base, and if any tag contains it, a reachable + `v*` tag whose own meta.yaml + CHANGELOG deprecate it, ≥ the notice period + before, counted from max(section date, tag date)); `### Removed` of its own, + naming no skill, for a removed adapter; one `### Removed` bullet naming skill + and agent for an agent dropped from a skill; `### Deprecated` for a newly + deprecated skill; `### Changed` naming skill + new version when a skill's + major goes up; `**Breaking:**` + `schema_version` for a catalog schema bump. + Urgent security removals skip deprecation/notice only with a `### Removed` + bullet marked `**Security:**` plus a `### Security` entry naming the skill. + Without release tags it prints a note and skips notice rules. It also fails + (as does `prepare_release.py`) when the newest dated section is a patch + release with Removed/Deprecated/**Breaking** entries. Run it locally with + `--base origin/main` (after `git fetch --tags`) before pushing. - Release CI must build once, verify wheel/sdist/plugin identity (against the tagged commit's files), produce a runtime-only SPDX SBOM and exact checksums, attest those bytes, then publish the same bundle. The build job diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2abe8a1..09ade8d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -56,7 +56,8 @@ skill's own `version` in `meta.yaml` whenever its content changes. Deprecating or removing a skill, dropping an agent, or making a major version change needs a `CHANGELOG.md` entry that names it, and CI checks for one. See [docs/lifecycle.md](docs/lifecycle.md) for the rules and the notice period, and run -`uv run --extra dev python scripts/check_lifecycle.py --base origin/main` to check. +`uv run --locked --extra dev python scripts/check_lifecycle.py --base origin/main` to +check. ## Opening the pull request diff --git a/SECURITY.md b/SECURITY.md index 613d96f..5c0081f 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -19,7 +19,9 @@ When you report, include as much of the following as you can: - We aim to acknowledge a report within **3 business days**. - We'll confirm the issue, keep you updated as we work on a fix, and let you know when - it ships. + it ships. For a confirmed high- or critical-severity issue, we aim to publish a fixed + release within 7 days of confirming it, as the + [lifecycle policy](docs/lifecycle.md#security-fixes) describes. - With your permission, we're happy to credit you once the fix is public. Please give us a reasonable window to release a fix before disclosing publicly. We're a diff --git a/docs/authoring-skills.md b/docs/authoring-skills.md index aedc938..d6d542e 100644 --- a/docs/authoring-skills.md +++ b/docs/authoring-skills.md @@ -82,7 +82,8 @@ they write one, and `skilldeck catalog --json` reports the record to tools Deprecating a skill also needs a `### Deprecated` entry in `CHANGELOG.md` naming it in backticks, and CI checks for one. The skill can be removed only after a release has published the deprecation for the notice period (90 -days before 1.0). A rename is a new skill plus a deprecation of the old name. +days before 1.0). While a skill is 0.x, a breaking change bumps its minor +version. A rename is a new skill plus a deprecation of the old name. See [Lifecycle and compatibility](lifecycle.md#deprecating-a-skill) for the full path, including what happens to installed copies, and [Skill versions](lifecycle.md#skill-versions) for which changes are major, diff --git a/docs/lifecycle.md b/docs/lifecycle.md index d7f1853..3641d82 100644 --- a/docs/lifecycle.md +++ b/docs/lifecycle.md @@ -14,11 +14,11 @@ A CI check enforces the parts a script can check (see | Change | Package release (before 1.0 / from 1.0) | Notice | CHANGELOG entry | |---|---|---|---| | Fix a skill's wording, typos or citations | patch | none | Changed or Fixed | -| Add a skill, agent, format, command, option or catalog field | patch or minor / minor | none | Added | +| Add a skill, agent, format, command, option or catalog field | minor / minor | none | Added | | Deprecate a skill, agent, format, command or option | minor / minor | none: this starts the notice period | Deprecated (checked for skills) | -| Remove a skill | minor / major | deprecation published at least 90 days earlier (180 from 1.0) (checked) | Removed (checked) | +| Remove a skill | minor / major | deprecation published at least 90 days earlier (180 from 1.0; a maintainer policy choice) (checked) | Removed (checked) | | Remove an agent or format, or drop an agent from one skill | minor / major | the same, unless the vendor removed it first | Removed (checked) | -| Make a breaking change to a skill (a major skill version) | minor / minor | none: read the entry before `update` | Changed, with the new version (checked) | +| Make a breaking change to a skill (a minor bump while the skill is 0.x, a major one from 1.0.0) | minor / minor | none: read the entry before `update` | Changed, with the new version (checked for a new major) | | Change the stamp format | minor / minor | none: old stamps stay readable | Changed | | Bump the catalog's `schema_version` | minor / major | none | **Breaking:** (checked) | | Fix an urgent security problem | whatever the change needs | may skip all of it | Security, plus an advisory | @@ -44,10 +44,11 @@ A skill's guidance is versioned by the skill itself (see [Skill versions](#skill-versions)), not by the package. **Before 1.0 (now)**, a breaking change bumps the minor version (`0.4.0` → -`0.5.0`). A deprecation also needs at least a minor release. New features and -fixes can go in either kind of release. So a patch release (`0.4.0` → -`0.4.1`) never removes, deprecates or breaks anything, and is always safe to -take. +`0.5.0`). A deprecation, or a new skill, agent, format, command, option or +catalog field, also needs at least a minor release. Fixes can go in either +kind of release. So a patch release (`0.4.0` → `0.4.1`) only fixes things: +it never adds a feature, or removes, deprecates or breaks anything, and is +always safe to take. A change is breaking if it does any of these: @@ -67,26 +68,42 @@ The release check holds releases to this. The newest dated CHANGELOG section can't be a patch release if it has `### Removed` or `### Deprecated` entries, or an entry marked **Breaking:**. From 1.0, Removed and Breaking entries need a major release. `scripts/prepare_release.py` refuses such a version before -it writes anything. +it writes anything. Reviewers check the rest, such as a new feature slipped +into a patch release. + +`### Removed` is only for removals from the public surface above: skills, +agents and adapters, commands and options, fields of the stable output +formats, and `meta.yaml` fields. Removing internal code, such as an unused +helper function, is a `### Changed` entry, so it doesn't force a minor +release. ### Skill versions Each skill's `version` in `meta.yaml` is SemVer too, applied to what the skill -tells the agent to do. Skills don't get SemVer's 0.x exception: the first -breaking change to a 0.x skill takes it to 1.0.0. +tells the agent to do. Like the package, a skill gets SemVer's 0.x exception: +while it is 0.x, a change that would be major bumps its minor version +(`0.4.0` → `0.5.0`). From 1.0.0 it follows SemVer fully. So a shared change, +such as a new severity rubric in every review skill, doesn't push every 0.x +skill to 1.0.0. -| Bump | When the change | Examples | +| Bump (from 1.0.0) | When the change | Examples | |---|---|---| | major | takes something away, or changes the shape of the result | removing a checklist area or a kind of finding; narrowing what the skill reviews; handing an area to another skill in [Which skill owns what](finding-output.md#which-skill-owns-what); changing the output shape (the finding format, severity rubric or report header) | | minor | adds to what the skill does, without taking anything away | new checks or a new checklist area; new sources that add checks; covering another kind of file; deprecating the skill; changing its `category`, which moves it between `catalog --category` filters | | patch | leaves what it reports unchanged | wording, typos, clarifications, citation and link fixes, a clearer worked example, a new `description` | +While a skill is 0.x, read "major" in this table as a minor bump, and "minor" +as a minor or patch bump. + Adding an agent to `supported-agents` is a patch. Dropping one follows [Dropping an agent from a skill](#dropping-an-agent-from-a-skill). -A major skill version needs at least a minor package release. It also needs a -`### Changed` entry naming the skill and its new version (checked), saying -what changed and what users should do. +A breaking skill change needs at least a minor package release, and a +`### Changed` entry naming the skill and its new version, saying what changed +and what users should do. The release check requires that entry when the +skill's major version goes up (0.x → 1.0.0, 1.x → 2.0.0); for a 0.x skill's +breaking minor bump, and for the package release it lands in, it is a +reviewer's job. How skill versions meet `status` and `update`: @@ -99,7 +116,7 @@ How skill versions meet `status` and `update`: sides. - `skilldeck update` rewrites every stale, unedited install, whatever the size of the bump. It never overwrites a locally modified install without - `--force`. Check the CHANGELOG for major skill versions before you run it. + `--force`. Check the CHANGELOG for breaking skill changes before you run it. To stay on the old guidance for a while, keep running the older skilldeck release (`uvx skilldeck@X.Y.Z`), or edit the installed file, which `update` then leaves alone. @@ -147,11 +164,17 @@ unchanged, so the CHANGELOG is where they learn about the deprecation. The notice period has two parts: -- the deprecation must ship in at least one **published** release, meaning a - dated CHANGELOG section with its `vX.Y.Z` tag; -- the removal can merge no sooner than **90 days** after that release's date - (**180 days** from 1.0). Before 1.0 it goes in a minor release, and from - 1.0 in a major one. +- the deprecation must ship in at least one **published** release: a + `vX.Y.Z` tag on `main` whose own files mark the skill `deprecated` and whose + own CHANGELOG has a `### Deprecated` entry naming it; +- the removal can merge no sooner than **90 days** after that release (**180 + days** from 1.0), counted from the later of its CHANGELOG date and its tag's + date. Before 1.0 it goes in a minor release, and from 1.0 in a major one. + +The 90 and 180 days, like the 7-day target under +[Security fixes](#security-fixes), are the maintainers' policy choices rather +than anything an outside standard sets. Changing one means changing this page +and `scripts/check_lifecycle.py` together. Why these numbers: @@ -173,10 +196,12 @@ To remove the skill: The release check also requires the skill to have been deprecated at the base of the pull request. If any release tag contains the skill, it also -requires the deprecation to have been published for the notice period. A -skill that no release ever contained must still be deprecated first, but it -has no waiting period. [Security fixes](#security-fixes) can skip all of -this. +requires a release to have published the deprecation for the notice period. +It reads that from the release tags themselves, not from the pull request's +CHANGELOG, so a Deprecated entry added to an old section afterwards doesn't +count. A skill that no release ever contained must still be deprecated first, +but it has no waiting period. [Security fixes](#security-fixes) can skip all +of this. A removed skill is gone from `list` and from the catalog. The catalog has no "removed" state: the CHANGELOG's Removed entry is the record, and the release @@ -268,10 +293,11 @@ steps as for a skill: 2. **Wait** for the same notice period: 90 days after the release that published the deprecation (180 from 1.0). 3. **Remove** it from `ADAPTERS` or `LEGACY_ADAPTERS`, along with its - contract fixtures and matrix row. Add a `### Removed` entry naming it - (checked) that lists the paths it installed to. If a successor format - exists, keep the old one in `MIGRATIONS` so that `migrate` can still move - installs. + contract fixtures and matrix row. Add a `### Removed` entry of its own + that names the adapter and no skill (checked), and lists the paths it + installed to. An entry saying one skill no longer supports the agent + doesn't count. If a successor format exists, keep the old one in + `MIGRATIONS` so that `migrate` can still move installs. After the removal, skilldeck rejects `--agent `, so `status` and `uninstall` can no longer see that adapter's files. Remove them before you @@ -314,10 +340,10 @@ fails, is breaking for skill authors. The one lifecycle field is `deprecated`, with `since`, `reason` and an optional `replacement`. It deliberately has no removal date or version. The -notice period starts when a release publishes the deprecation, and that date -is known only from the CHANGELOG and the release tag, which the release check -reads. A date written into `meta.yaml` in advance would be a guess that could -disagree with them. +notice period starts when a release publishes the deprecation, and only the +release tag knows that: its date, its files and its CHANGELOG, which the +release check reads. A date written into `meta.yaml` in advance would be a +guess that could disagree with them. ### The catalog @@ -388,21 +414,17 @@ as described here. ### Lockfiles and install state Today the install state is the stamped files themselves. There is no lockfile -or install database. [#71](https://github.com/IcebergAI/skilldeck/issues/71) -will add lockfiles and bundles, and they will follow this page: +or install database. When [#71](https://github.com/IcebergAI/skilldeck/issues/71) +adds lockfiles, they follow these rules (what a lockfile records is #71's +decision): - A lockfile carries its own integer `schema_version`, under the catalog's - additive and breaking rules. Like stamps, every lockfile version skilldeck - has written stays readable. -- It records identities the catalog already publishes: the skill's name, - version and `canonical_sha256`, plus the adapter, the scope and the - `rendered_sha256` for each agent (the stamp's `hash=`). `rendered_sha256` - can change with the rendering while the skill version stays the same, so a - lockfile that pins it is also pinned to a skilldeck version. -- A locked skill that becomes deprecated keeps installing, with the usual - warning, through the notice period. A locked skill, agent or format that - has been removed fails before anything is written, with an error that names - it and points to the CHANGELOG. + additive and breaking rules. +- Like stamps, every lockfile version skilldeck has written stays readable. +- A locked skill, agent or format that has been removed fails clearly, + before anything is written, with an error that names it and points to the + CHANGELOG. A deprecated one keeps working through the notice period, with + the usual warning. Lockfiles aren't a stable contract until they ship under these rules. @@ -417,10 +439,12 @@ These are stable and covered by the package version: [adapter contracts](compatibility.md#contract-tests); - `skilldeck catalog --json` and `--schema` (see [the catalog](catalog.md)); - `skilldeck provenance --json` (`schema_version` 1), under the same additive - and breaking rules as the catalog; -- eval run records (`evals/run-record.schema.json`, `schema_version` 1), - under the same rules. They belong to the repository's eval tooling rather - than the package. + and breaking rules as the catalog. + +Eval run records are not part of the package: they come from the +repository's eval tooling (`evals/run_evals.py`). Their stability follows +their own `schema_version` (`evals/run-record.schema.json`, currently 1) +under the catalog's additive and breaking rules, not the package version. Human-readable output isn't stable, and can change in any release without notice. That covers `list`, `status`, `update`, `install`, `uninstall`, @@ -454,14 +478,21 @@ It must still: - add a `### Security` entry that names what is affected in backticks, gives the affected versions, and says what users should do (for example `skilldeck update`, or `skilldeck uninstall --agent `), along - with the usual Removed or Changed entry. For a skill removal, the Security - entry naming the skill is what lets the release check skip the deprecation - and notice requirements; + with the usual Removed or Changed entry; +- for a skill removed this way, start its `### Removed` entry with + `**Security:**`, for example + ``- **Security:** `old-review`, which told agents to skip TLS checks.`` + The release check skips the deprecation and notice requirements only for a + Removed entry with that mark and a Security entry naming the same skill, both + added by the pull request. A Security entry that merely mentions a skill + doesn't exempt its removal; - for a vulnerability in skilldeck itself, publish a GitHub security advisory, handling the report as [SECURITY.md](../SECURITY.md) describes. Fixes ship on the latest release only, with no backports. For a confirmed -high- or critical-severity issue, the aim is a release within 7 days. +high- or critical-severity issue, the maintainers aim for a release within 7 +days. That target is their policy choice, like the notice periods, and +[SECURITY.md](../SECURITY.md) states it too. ## Release checks @@ -469,27 +500,40 @@ high- or critical-severity issue, the aim is a release within 7 days. `--base origin/`, comparing the pull request with its target. It fails unless `CHANGELOG.md` has: -- for a **removed skill**, a `### Removed` entry naming it. Unless a - `### Security` entry names it too, the skill must have been deprecated at - the base. If a release tag contains the skill, a `### Deprecated` entry - naming it must also be in a tagged, dated section at least 90 days old (180 - from 1.0); +- for a **removed skill**, a `### Removed` entry naming it. The skill must + also have been deprecated at the base, and if a release tag contains it, a + release must have published the deprecation at least 90 days earlier (180 + from 1.0): a `vX.Y.Z` tag reachable from the base whose own `meta.yaml` + marks the skill deprecated and whose own CHANGELOG has a `### Deprecated` + entry naming it, counted from the later of that section's date and the + tag's date. The [urgent security path](#security-fixes) skips both + requirements; - for a **newly deprecated skill**, a `### Deprecated` entry naming it; - for an **agent dropped from a skill's `supported-agents`**, a single `### Removed` entry naming both the skill and the agent. If the agent's adapter is gone altogether, the next rule covers it instead; - for a **removed adapter** (listed in the base's `tests/fixtures/adapter-contracts/contracts.json` but no longer in - `ALL_ADAPTERS`), a `### Removed` entry naming it; -- for a **major skill version**, a `### Changed`, `### Removed` or - `### Security` entry naming the skill and giving its new version; + `ALL_ADAPTERS`), a `### Removed` entry of its own that names it and no + skill; +- for a **new major skill version** (0.x → 1.0.0, 1.x → 2.0.0), a + `### Changed`, `### Removed` or `### Security` entry naming the skill and + giving its new version; - for a **catalog `schema_version` change**, an entry marked **Breaking:** that mentions `schema_version`. -Entries count only under `## [Unreleased]` or in the newest dated section, -which is where cutting a release moves them. They must name each skill, -agent or adapter in backticks, like `` `old-review` ``. Every error says what -is missing, where to add it, and which section of this page applies. +Entries count only in sections the pull request adds: `## [Unreleased]`, or +a dated section the base's CHANGELOG doesn't have yet, which is where cutting +a release in the same pull request moves them. A section an earlier release +published doesn't count. Entries must name each skill, agent or adapter in +backticks of its own, like `` `old-review` ``: a name inside a longer code +span, such as a command, doesn't count. Every error says what is missing, +where to add it, and which section of this page applies. + +The notice rules need the release tags. CI's `lint` job fetches them; in a +checkout without any `vX.Y.Z` tag reachable from the base, the script prints +`note: no release tags found; notice-period rules skipped` with a reminder to +run `git fetch --tags`, and checks everything else. With or without `--base`, the script also applies the version-bump rule from [The package](#the-package) to the newest dated section. On a push to `main` @@ -505,6 +549,10 @@ These stay with reviewers: - whether a skill change is major, minor or patch (the check only sees the number); +- that a breaking skill change comes with at least a minor package release, + and, for a 0.x skill's breaking minor bump, a Changed entry; +- that a new feature isn't released in a patch release, and that + `### Removed` holds only public-surface removals; - the deprecation and notice period for removing an adapter, a format, a command or an option, or dropping an agent from a skill, where a vendor's move can make the notice moot; diff --git a/docs/releasing.md b/docs/releasing.md index 965632d..695d4d0 100644 --- a/docs/releasing.md +++ b/docs/releasing.md @@ -9,10 +9,10 @@ rules can't silently drift. - **Project version** lives in `pyproject.toml` `[project].version` and is the single source of truth; `uv.lock` mirrors it (run `uv lock` after a bump). - **SemVer**, and the project is **pre-1.0**: a breaking change bumps the - **minor** (`0.2 → 0.3`); features and fixes bump the minor or patch at - discretion. (Dropping Python 3.9 in 0.2.0 was breaking; the two new skills in - 0.3.0 were additive.) A release with Removed or Deprecated entries, or an - entry marked **Breaking:**, can't be a patch release. See + **minor** (`0.2 → 0.3`), and so does a new feature; fixes bump the minor or + patch at discretion. (Dropping Python 3.9 in 0.2.0 was breaking; the two new + skills in 0.3.0 were additive.) A release with Removed or Deprecated + entries, or an entry marked **Breaking:**, can't be a patch release. See [Lifecycle and compatibility](lifecycle.md#the-package) for what counts as breaking, the rules from 1.0, and the notice period before a removal. - **Skill versions are independent.** Each skill carries its own `version` in @@ -188,9 +188,11 @@ Consumer verification is documented in and the package, so the `lint` job runs it with `uv run`. On a pull request it compares the change with `--base origin/`, and requires a CHANGELOG entry for each removed or newly deprecated skill, agent dropped -from a skill, removed adapter, major skill version and catalog -`schema_version` change. A skill removal must also follow a published -deprecation by the notice period. Every run also checks that the newest +from a skill, removed adapter, new major skill version and catalog +`schema_version` change. A skill removal must also follow a deprecation that +a release tag published at least the notice period earlier, so the check +needs the tags that `fetch-depth: 0` brings; without any, it prints a note +and skips that rule. Every run also checks that the newest dated CHANGELOG section is a big enough version bump for its Removed, Deprecated and **Breaking:** entries, which `prepare_release.py` checks before it writes anything. diff --git a/scripts/check_lifecycle.py b/scripts/check_lifecycle.py index f2e6d20..b9820d7 100644 --- a/scripts/check_lifecycle.py +++ b/scripts/check_lifecycle.py @@ -5,26 +5,31 @@ machine can. With ``--base `` (a PR's target branch; CI's ``lint`` job passes ``origin/``), it compares the working tree with that ref: -- a skill directory removed: a ``### Removed`` entry naming the skill. Unless a - ``### Security`` entry names it too (the urgent path), the skill must be - ``deprecated`` in its ``meta.yaml`` at the base and, if any release tag - contains the skill, a ``### Deprecated`` entry naming it must sit in a - published release (a dated section whose ``v`` tag exists) dated at - least ``NOTICE_DAYS`` ago (``NOTICE_DAYS_STABLE`` from 1.0); +- a skill directory removed: a ``### Removed`` entry naming the skill. The + skill must also be ``deprecated`` in its ``meta.yaml`` at the base and, if a + release tag contains the skill, a release must have published the + deprecation at least ``NOTICE_DAYS`` ago (``NOTICE_DAYS_STABLE`` from 1.0): + a tag whose own ``meta.yaml`` marks the skill deprecated and whose own + CHANGELOG has a ``### Deprecated`` entry naming it, counted from the later of + that section's date and the tag's date. The urgent security path skips the + deprecation and the notice: the ``### Removed`` entry is marked + ``**Security:**`` and a ``### Security`` entry names the skill too; - a skill newly ``deprecated``: a ``### Deprecated`` entry naming it; -- a skill no longer listing an agent in ``supported-agents``: a ``### Removed`` - entry naming the skill and the agent (an agent whose adapter is gone - altogether is covered by the next rule instead); +- a skill no longer listing an agent in ``supported-agents``: one + ``### Removed`` entry naming the skill and the agent (an agent whose adapter + is gone altogether is covered by the next rule instead); - an adapter removed (named in the base's adapter contracts, missing from - ``ALL_ADAPTERS`` now): a ``### Removed`` entry naming it; + ``ALL_ADAPTERS`` now): a ``### Removed`` entry naming it and no skill; - a skill's major version raised: a ``### Changed``, ``### Removed`` or ``### Security`` entry naming the skill and its new version; - the catalog's ``schema_version`` changed: a ``**Breaking:**`` entry that mentions ``schema_version``. -Each entry must be a bullet in ``## [Unreleased]`` or in the newest dated -section (cutting a release moves ``[Unreleased]`` there), and must name the -skill, agent or adapter in backticks, e.g. `` `old-review` ``. +Each entry must be a bullet in a section this change adds: ``## [Unreleased]``, +or a dated section the base's CHANGELOG does not have yet (a release cut in +the same change). It must name the skill, agent or adapter in backticks of its +own, e.g. `` `old-review` ``. Without release tags the notice rules can't be +checked; the script says so and carries on. With or without ``--base`` it also checks the newest dated CHANGELOG section's version against the one before it: a section with ``### Removed`` or @@ -43,7 +48,7 @@ import re import subprocess import sys -from collections.abc import Iterable +from collections.abc import Collection, Iterable from dataclasses import dataclass, field from pathlib import Path @@ -66,6 +71,12 @@ CONTRACTS_PATH = "tests/fixtures/adapter-contracts/contracts.json" CATALOG_SCHEMA_PATH = "src/skilldeck/catalog.schema.json" POLICY = "docs/lifecycle.md" +#: marks the ``### Removed`` entry of an urgent security removal +SECURITY_MARK = "**Security:**" +NO_TAGS_NOTE = ( + "no release tags found; notice-period rules skipped — run " + "`git fetch --tags` if there should be some" +) _SECTION_RE = re.compile( r"## \[(?PUnreleased|\d+\.\d+\.\d+)\]" @@ -75,6 +86,10 @@ _ENTRY_RE = re.compile(r"^[-*] ", re.MULTILINE) +class ChangelogError(ValueError): + """A CHANGELOG this script cannot read.""" + + # --- the CHANGELOG ------------------------------------------------------------ @@ -92,12 +107,13 @@ def entries(self, *groups: str) -> list[str]: return [entry for name in names for entry in self.groups.get(name, [])] -def parse_changelog(text: str) -> list[Section]: +def parse_changelog(text: str, source: str = "CHANGELOG.md") -> list[Section]: """Every ``## [Unreleased]`` or ``## [x.y.z]`` section, in file order. Entries are the top-level ``-`` bullets, each with its indented continuation lines and sub-bullets. Group names are title-cased, so - ``### removed`` counts as ``Removed``. + ``### removed`` counts as ``Removed``. A section dated with a day that + does not exist raises :class:`ChangelogError` naming ``source``. """ sections: list[Section] = [] text = text.replace("\r\n", "\n") @@ -107,9 +123,15 @@ def parse_changelog(text: str) -> list[Section]: continue # the preamble, or a heading this parser does not know version = header.group("version") date = header.group("date") + try: + day = datetime.date.fromisoformat(date) if date else None + except ValueError: + raise ChangelogError( + f"{source}: `{header.group(0)}` has an invalid date; use a real " + "YYYY-MM-DD day" + ) from None section = Section( - version=None if version == "Unreleased" else version, - date=datetime.date.fromisoformat(date) if date else None, + version=None if version == "Unreleased" else version, date=day ) parts = _GROUP_RE.split(chunk) # parts: [before the first group, name, body, name, body, ...] @@ -120,16 +142,19 @@ def parse_changelog(text: str) -> list[Section]: return sections -def recent_sections(sections: list[Section]) -> list[Section]: - """``[Unreleased]`` and the newest dated section. +def recent_sections( + sections: list[Section], base_versions: Collection[str] +) -> list[Section]: + """The sections a change adds: ``[Unreleased]``, and any dated section + whose version the base's CHANGELOG (``base_versions``) does not have. - A note belongs in ``[Unreleased]``; cutting a release - (``scripts/prepare_release.py``) moves it into a new dated section and - leaves ``[Unreleased]`` empty, so the newest dated section counts too. + A note belongs in ``[Unreleased]``. A change that also cuts a release + (``scripts/prepare_release.py``) moves it into a new dated section, so + that section counts too; one the base already has was published before + and does not. """ recent = [s for s in sections if s.version is None][:1] - dated = _dated(sections) - return [*recent, *dated[-1:]] + return recent + [s for s in _dated(sections) if s.version not in base_versions] def _dated(sections: list[Section]) -> list[Section]: @@ -152,12 +177,23 @@ def mentions_version(entry: str, version: str) -> bool: return re.search(pattern, entry) is not None -def _recorded(sections: list[Section], groups: tuple[str, ...], *terms: str) -> bool: - return any( - names(entry, *terms) - for section in recent_sections(sections) - for entry in section.entries(*groups) +def _entries(recent: list[Section], *groups: str) -> list[str]: + return [entry for section in recent for entry in section.entries(*groups)] + + +def _recorded(recent: list[Section], groups: tuple[str, ...], *terms: str) -> bool: + return any(names(entry, *terms) for entry in _entries(recent, *groups)) + + +def urgent_removal(recent: list[Section], name: str) -> bool: + """Whether skill ``name`` is removed on the urgent security path: its + ``### Removed`` entry is marked ``**Security:**`` and a ``### Security`` + entry names it too.""" + marked = any( + names(entry, name) and SECURITY_MARK in entry + for entry in _entries(recent, "Removed") ) + return marked and _recorded(recent, ("Security",), name) # --- skills, adapters and the catalog at a ref or in the working tree --------- @@ -256,31 +292,79 @@ def _schema_version(text: str | None) -> object: return None -def _release_tags() -> list[str]: - """Every exact ``vX.Y.Z`` tag (as a full ref).""" - refs = _git("for-each-ref", "--format=%(refname)", "refs/tags/v*").stdout +# --- release tags --------------------------------------------------------------- + + +@dataclass(frozen=True) +class Tag: + """A ``vX.Y.Z`` release tag reachable from the base.""" + + ref: str + date: datetime.date | None # the tag's own date (a lightweight tag's commit's) + + @property + def name(self) -> str: + return self.ref.removeprefix("refs/tags/") + + +def release_tags(base: str) -> list[Tag]: + """Every exact ``vX.Y.Z`` tag reachable from ``base``: a release tag must + be on ``main``, so one elsewhere was never a release.""" + listing = _git( + "for-each-ref", + f"--merged={base}", + "--format=%(refname) %(creatordate:short)", + "refs/tags/v*", + ).stdout.decode("utf-8") tags = [] - for ref in refs.decode("utf-8").split(): + for line in listing.splitlines(): + ref, _, day = line.partition(" ") try: consistency.normalize_tag(ref) except ValueError: continue - tags.append(ref) + try: + date = datetime.date.fromisoformat(day) + except ValueError: + date = None + tags.append(Tag(ref, date)) return tags -def _tag_exists(version: str) -> bool: - ref = f"refs/tags/v{version}^{{commit}}" - return _git("rev-parse", "-q", "--verify", ref).returncode == 0 +def released(name: str, tags: Iterable[Tag]) -> bool: + """Whether any of ``tags`` contains skill ``name``.""" + path = f"{SKILLS_PATH}/{name}/meta.yaml" + return any(_git("cat-file", "-e", f"{t.ref}:{path}").returncode == 0 for t in tags) -def released(name: str) -> bool: - """Whether any ``vX.Y.Z`` release tag contains skill ``name``.""" - path = f"{SKILLS_PATH}/{name}/meta.yaml" - return any( - _git("cat-file", "-e", f"{tag}:{path}").returncode == 0 - for tag in _release_tags() - ) +def deprecation_published( + name: str, tags: Iterable[Tag] +) -> tuple[datetime.date, Tag] | None: + """When a release first published skill ``name``'s deprecation, and which. + + A tag counts only if its own ``meta.yaml`` marks the skill deprecated and + its own CHANGELOG has a dated ``### Deprecated`` entry naming it; the + notice starts at the later of that section's date and the tag's date, so + neither a Deprecated entry added to an old section afterwards nor an old + section date on a late tag shortens it. + """ + starts = [] + for tag in tags: + meta = _git_text(tag.ref, f"{SKILLS_PATH}/{name}/meta.yaml") + log = _git_text(tag.ref, "CHANGELOG.md") + if meta is None or log is None or tag.date is None: + continue + if not skill_facts(meta).deprecated: + continue + dates = [ + section.date + for section in _dated(parse_changelog(log, f"{tag.name}:CHANGELOG.md")) + if section.date is not None + and any(names(entry, name) for entry in section.entries("Deprecated")) + ] + if dates: + starts.append((max(min(dates), tag.date), tag)) + return min(starts, key=lambda start: start[0]) if starts else None # --- the rules ----------------------------------------------------------------- @@ -305,33 +389,23 @@ def notice_days() -> int: return NOTICE_DAYS if major == 0 else NOTICE_DAYS_STABLE -def _notice_error( - name: str, sections: list[Section], today: datetime.date, days: int -) -> str: +def _notice_error(name: str, tags: list[Tag], today: datetime.date, days: int) -> str: """Why removing skill ``name`` today breaks the notice rule, or ``""``.""" - announced = [ - section - for section in sections - if section.version is not None - and section.date is not None - and any(names(entry, name) for entry in section.entries("Deprecated")) - and _tag_exists(section.version) - ] - if not announced: + published = deprecation_published(name, tags) + if published is None: return ( - f"skill `{name}` was removed, but no published release announces " - f"its deprecation: a `### Deprecated` entry naming `{name}` must " - f"ship in a tagged release at least {days} days before the " - f"removal ({POLICY}#removing-a-skill)" + f"skill `{name}` was removed, but no release published its " + "deprecation: a tagged release must mark it `deprecated` and have " + f"a `### Deprecated` entry naming `{name}` at least {days} days " + f"before the removal ({POLICY}#removing-a-skill)" ) - first = min(announced, key=lambda s: s.date or today) - assert first.date is not None and first.version is not None - allowed = first.date + datetime.timedelta(days=days) + start, tag = published + allowed = start + datetime.timedelta(days=days) if today < allowed: return ( - f"skill `{name}` was removed too early: its deprecation was " - f"published in {first.version} on {first.date.isoformat()}, so it " - f"can be removed from {allowed.isoformat()} ({days} days' notice; " + f"skill `{name}` was removed too early: {tag.name} published its " + f"deprecation on {start.isoformat()}, so it can be removed from " + f"{allowed.isoformat()} ({days} days' notice; " f"{POLICY}#removing-a-skill)" ) return "" @@ -349,34 +423,39 @@ def _major(version: str) -> int: def skill_errors( base: dict[str, SkillFacts], head: dict[str, SkillFacts], - sections: list[Section], + recent: list[Section], adapters: Iterable[str], today: datetime.date, + tags: list[Tag], days: int = NOTICE_DAYS, ) -> list[str]: - """Lifecycle notes missing for the skill changes from ``base`` to ``head``, - with ``days`` of notice for a removal.""" + """Lifecycle notes missing for the skill changes from ``base`` to ``head``. + + ``recent`` are the CHANGELOG sections the change adds, ``tags`` the + release tags reachable from the base, and ``days`` the notice period. + """ adapters = set(adapters) errors = [] for name in sorted(base.keys() - head.keys()): - if not _recorded(sections, ("Removed",), name): + if not _recorded(recent, ("Removed",), name): errors.append( f"skill `{name}` was removed: {_where('Removed', name)}, saying " "what replaces it and how to delete installed copies " f"({POLICY}#removing-a-skill)" ) - if _recorded(sections, ("Security",), name): + if urgent_removal(recent, name): continue # the urgent path: no deprecation or notice period if not base[name].deprecated: errors.append( f"skill `{name}` was removed without being deprecated first: " "mark it `deprecated` in its meta.yaml, release that, and " f"remove it after the notice period ({POLICY}#removing-a-skill). " - f"An urgent security removal instead needs a `### Security` " - f"entry naming `{name}` ({POLICY}#security-fixes)" + f"An urgent security removal instead marks its `### Removed` " + f"entry `{SECURITY_MARK}` and adds a `### Security` entry " + f"naming `{name}` ({POLICY}#security-fixes)" ) - elif released(name): - notice = _notice_error(name, sections, today, days) + elif released(name, tags): + notice = _notice_error(name, tags, today, days) if notice: errors.append(notice) for name in sorted(base.keys() & head.keys()): @@ -384,7 +463,7 @@ def skill_errors( if old.version is None or new.version is None: continue # unreadable meta.yaml: the registry's tests report it newly_deprecated = new.deprecated and not old.deprecated - if newly_deprecated and not _recorded(sections, ("Deprecated",), name): + if newly_deprecated and not _recorded(recent, ("Deprecated",), name): errors.append( f"skill `{name}` is newly deprecated: " f"{_where('Deprecated', name)}, naming its replacement or the " @@ -393,7 +472,7 @@ def skill_errors( for agent in sorted(old.agents - new.agents): if agent not in adapters: continue # the adapter itself is gone: adapter_errors covers it - if not _recorded(sections, ("Removed",), name, agent): + if not _recorded(recent, ("Removed",), name, agent): errors.append( f"skill `{name}` no longer supports `{agent}`: " f"{_where('Removed', name, agent)}, " @@ -402,8 +481,7 @@ def skill_errors( ) if _major(new.version) > _major(old.version) and not any( names(entry, name) and mentions_version(entry, new.version) - for section in recent_sections(sections) - for entry in section.entries("Changed", "Removed", "Security") + for entry in _entries(recent, "Changed", "Removed", "Security") ): errors.append( f"skill `{name}` went from {old.version} to {new.version}, a " @@ -416,28 +494,39 @@ def skill_errors( def adapter_errors( - base: set[str] | None, head: Iterable[str], sections: list[Section] + base: set[str] | None, + head: Iterable[str], + recent: list[Section], + skills: Collection[str], ) -> list[str]: - """Lifecycle notes missing for adapters removed since ``base``.""" + """Lifecycle notes missing for adapters removed since ``base``. + + The entry must name the adapter and none of ``skills``, so a per-skill + "``x`` no longer supports ``agent``" entry doesn't count as the note that + the adapter itself is gone. + """ if base is None: return [] return [ - f"adapter `{name}` was removed: {_where('Removed', name)}, listing the " - "paths it installed to so users can delete what is left " - f"({POLICY}#removing-an-agent-or-format)" + f"adapter `{name}` was removed: add a `### Removed` entry of its own " + f"naming `{name}` (in backticks) and no skill under `## [Unreleased]` " + "in CHANGELOG.md, listing the paths it installed to so users can " + f"delete what is left ({POLICY}#removing-an-agent-or-format)" for name in sorted(base - set(head)) - if not _recorded(sections, ("Removed",), name) + if not any( + names(entry, name) and not any(names(entry, s) for s in skills) + for entry in _entries(recent, "Removed") + ) ] -def catalog_errors(base: object, head: object, sections: list[Section]) -> list[str]: +def catalog_errors(base: object, head: object, recent: list[Section]) -> list[str]: """A catalog ``schema_version`` change needs a **Breaking:** entry.""" if base is None or head is None or base == head: return [] if any( "**Breaking" in entry and "`schema_version`" in entry - for section in recent_sections(sections) - for entry in section.entries() + for entry in _entries(recent) ): return [] return [ @@ -484,8 +573,14 @@ def release_bump_errors(sections: list[Section]) -> list[str]: ] -def lifecycle_errors(base: str, today: datetime.date | None = None) -> list[str]: - """Every lifecycle note the change from ``base`` to the working tree lacks.""" +def lifecycle_check( + base: str, today: datetime.date | None = None +) -> tuple[list[str], list[str]]: + """The lifecycle notes the change from ``base`` to the working tree lacks, + and any notes about what could not be checked. + + Raises :class:`ChangelogError` if a CHANGELOG it needs cannot be read. + """ if today is None: today = datetime.datetime.now(datetime.timezone.utc).date() try: @@ -493,21 +588,33 @@ def lifecycle_errors(base: str, today: datetime.date | None = None) -> list[str] except OSError: in_git = False if not in_git: - return [f"--base {base} needs a git checkout"] + return [f"--base {base} needs a git checkout"], [] if _git("rev-parse", "-q", "--verify", f"{base}^{{commit}}").returncode != 0: - return [f"base ref {base!r} not found (fetch it first)"] + return [f"base ref {base!r} not found (fetch it first)"], [] sections = parse_changelog((ROOT / "CHANGELOG.md").read_text(encoding="utf-8")) + base_log = _git_text(base, "CHANGELOG.md") + base_versions = { + s.version + for s in parse_changelog(base_log or "", f"{base}:CHANGELOG.md") + if s.version is not None + } + recent = recent_sections(sections, base_versions) + tags = release_tags(base) + base_skills, head_skills = skills_at(base), skills_now() head_schema = ROOT / CATALOG_SCHEMA_PATH - return [ + errors = [ *skill_errors( - skills_at(base), - skills_now(), - sections, + base_skills, + head_skills, + recent, ALL_ADAPTERS, today, + tags, notice_days(), ), - *adapter_errors(adapters_at(base), ALL_ADAPTERS, sections), + *adapter_errors( + adapters_at(base), ALL_ADAPTERS, recent, base_skills.keys() | head_skills + ), *catalog_errors( _schema_version(_git_text(base, CATALOG_SCHEMA_PATH)), _schema_version( @@ -515,9 +622,15 @@ def lifecycle_errors(base: str, today: datetime.date | None = None) -> list[str] if head_schema.is_file() else None ), - sections, + recent, ), ] + return errors, ([] if tags else [NO_TAGS_NOTE]) + + +def lifecycle_errors(base: str, today: datetime.date | None = None) -> list[str]: + """Every lifecycle note the change from ``base`` to the working tree lacks.""" + return lifecycle_check(base, today)[0] def main(argv: list[str] | None = None) -> int: @@ -529,17 +642,23 @@ def main(argv: list[str] | None = None) -> int: ) args = parser.parse_args(argv) - changelog = (ROOT / "CHANGELOG.md").read_text(encoding="utf-8") - errors = release_bump_errors(parse_changelog(changelog)) - if args.base: - errors += lifecycle_errors(args.base) + notes: list[str] = [] + try: + changelog = (ROOT / "CHANGELOG.md").read_text(encoding="utf-8") + errors = release_bump_errors(parse_changelog(changelog)) + if args.base: + found, notes = lifecycle_check(args.base) + errors += found + except ChangelogError as exc: + errors = [str(exc)] + for note in notes: + print(f"note: {note}", file=sys.stderr) if errors: for error in errors: print(f"error: {error}", file=sys.stderr) print( f"\nSee {POLICY}: a compatibility change needs a CHANGELOG entry " - "naming what\nchanged (in backticks), under [Unreleased] or the " - "newest dated section.", + "naming what\nchanged (in backticks), under [Unreleased].", file=sys.stderr, ) return 1 diff --git a/scripts/prepare_release.py b/scripts/prepare_release.py index 3f76cb2..8bc94c0 100644 --- a/scripts/prepare_release.py +++ b/scripts/prepare_release.py @@ -103,7 +103,11 @@ def plan(version: str, today: str, root: Path = ROOT) -> tuple[str, dict[Path, s new_pyproject, old = bump_pyproject(pyproject.read_text(encoding="utf-8"), version) new_changelog = cut_changelog(changelog.read_text(encoding="utf-8"), version, today) # removals, deprecations and breaking changes need a minor (or major) bump - bump = lifecycle.release_bump_errors(lifecycle.parse_changelog(new_changelog)) + try: + sections = lifecycle.parse_changelog(new_changelog) + except lifecycle.ChangelogError as exc: + raise SystemExit(f"error: {exc}") from None + bump = lifecycle.release_bump_errors(sections) if bump: raise SystemExit(f"error: {bump[0]}") return old, {pyproject: new_pyproject, changelog: new_changelog} diff --git a/tests/_schema.py b/tests/_schema.py new file mode 100644 index 0000000..9b1485f --- /dev/null +++ b/tests/_schema.py @@ -0,0 +1,121 @@ +"""A minimal JSON Schema validator for the tests, and the packaged catalog schema. + +Only the keywords the committed catalog schema uses, so the tests need no new +dependency; test_schema_uses_only_supported_keywords (tests/test_catalog.py) +keeps it honest. Shared by the catalog and lifecycle tests. +""" + +import json +import re + +from skilldeck.catalog import catalog_schema_text + +SUPPORTED_KEYWORDS = { + "$schema", + "title", + "description", + "$defs", + "$ref", + "type", + "const", + "required", + "properties", + "additionalProperties", + "oneOf", + "items", + "pattern", + "minLength", + "maxLength", + "minItems", + "uniqueItems", +} +_TYPES = { + "object": lambda v: isinstance(v, dict), + "array": lambda v: isinstance(v, list), + "string": lambda v: isinstance(v, str), + "null": lambda v: v is None, + "integer": lambda v: isinstance(v, int) and not isinstance(v, bool), + "boolean": lambda v: isinstance(v, bool), +} + + +def catalog_schema(): + return json.loads(catalog_schema_text()) + + +def schema_errors(instance, schema=None, *, exact=False): + """Every way ``instance`` breaks ``schema``; with ``exact``, also any + object property the schema does not describe.""" + root = schema or catalog_schema() + + def check(value, node, path, exact): + errors: list[str] = [] + if "$ref" in node: + # the schema's $ref siblings are only annotations, so merging the + # target in is exact here (and lets ``exact`` see its properties) + target = root["$defs"][node["$ref"].removeprefix("#/$defs/")] + node = {**target, **{k: v for k, v in node.items() if k != "$ref"}} + if "type" in node: + types = node["type"] if isinstance(node["type"], list) else [node["type"]] + if not any(_TYPES[t](value) for t in types): + return [f"{path}: {value!r} is not of type {types}"] + if "const" in node and ( + value != node["const"] or type(value) is not type(node["const"]) + ): + errors.append(f"{path}: {value!r} is not {node['const']!r}") + if "oneOf" in node: + # branches only constrain; ``exact`` is the enclosing node's job + matches = [ + not check(value, branch, path, False) for branch in node["oneOf"] + ] + if matches.count(True) != 1: + errors.append(f"{path}: matches {matches.count(True)} of oneOf") + if isinstance(value, str): + if len(value) < node.get("minLength", 0): + errors.append(f"{path}: too short") + if len(value) > node.get("maxLength", len(value)): + errors.append(f"{path}: too long") + if "pattern" in node and not re.search(node["pattern"], value): + errors.append(f"{path}: {value!r} does not match {node['pattern']}") + if isinstance(value, list): + if len(value) < node.get("minItems", 0): + errors.append(f"{path}: too few items") + if node.get("uniqueItems") and len(set(map(json.dumps, value))) != len( + value + ): + errors.append(f"{path}: items are not unique") + if "items" in node: + for index, item in enumerate(value): + errors += check(item, node["items"], f"{path}[{index}]", exact) + if isinstance(value, dict): + for key in node.get("required", []): + if key not in value: + errors.append(f"{path}: missing {key}") + properties = node.get("properties", {}) + for key, item in value.items(): + if key in properties: + errors += check(item, properties[key], f"{path}.{key}", exact) + elif "additionalProperties" in node: + extra = node["additionalProperties"] + errors += check(item, extra, f"{path}.{key}", exact) + elif exact: + errors.append(f"{path}: undocumented property {key}") + return errors + + return check(instance, root, "$", exact) + + +def schema_nodes(node, *, branches=True): + """``node`` and every schema below it; ``branches``: including oneOf's.""" + yield node + children = [ + *node.get("properties", {}).values(), + *node.get("$defs", {}).values(), + ] + for key in ("items", "additionalProperties"): + if isinstance(node.get(key), dict): + children.append(node[key]) + if branches: + children += node.get("oneOf", []) + for child in children: + yield from schema_nodes(child, branches=branches) diff --git a/tests/test_catalog.py b/tests/test_catalog.py index b8552a7..2ab4eb8 100644 --- a/tests/test_catalog.py +++ b/tests/test_catalog.py @@ -8,13 +8,13 @@ import pytest from click.testing import CliRunner +from _schema import SUPPORTED_KEYWORDS, catalog_schema, schema_errors, schema_nodes from skilldeck import __version__, catalog, provenance, registry from skilldeck.adapters import ADAPTERS from skilldeck.catalog import ( CATALOG_SCHEMA_VERSION, CatalogError, build_catalog, - catalog_schema_text, ) from skilldeck.cli import cli from skilldeck.provenance import ( @@ -25,123 +25,9 @@ from skilldeck.registry import DEFAULT_SKILLS_DIR, discover_skills from skilldeck.targets import Scope -# --- a minimal JSON Schema validator ------------------------------------------ -# Only the keywords the committed schema uses, so the tests need no new -# dependency; test_schema_uses_only_supported_keywords keeps it honest. - -SUPPORTED_KEYWORDS = { - "$schema", - "title", - "description", - "$defs", - "$ref", - "type", - "const", - "required", - "properties", - "additionalProperties", - "oneOf", - "items", - "pattern", - "minLength", - "maxLength", - "minItems", - "uniqueItems", -} -_TYPES = { - "object": lambda v: isinstance(v, dict), - "array": lambda v: isinstance(v, list), - "string": lambda v: isinstance(v, str), - "null": lambda v: v is None, - "integer": lambda v: isinstance(v, int) and not isinstance(v, bool), - "boolean": lambda v: isinstance(v, bool), -} - - -def _schema(): - return json.loads(catalog_schema_text()) - - -def schema_errors(instance, schema=None, *, exact=False): - """Every way ``instance`` breaks ``schema``; with ``exact``, also any - object property the schema does not describe.""" - root = schema or _schema() - - def check(value, node, path, exact): - errors: list[str] = [] - if "$ref" in node: - # the schema's $ref siblings are only annotations, so merging the - # target in is exact here (and lets ``exact`` see its properties) - target = root["$defs"][node["$ref"].removeprefix("#/$defs/")] - node = {**target, **{k: v for k, v in node.items() if k != "$ref"}} - if "type" in node: - types = node["type"] if isinstance(node["type"], list) else [node["type"]] - if not any(_TYPES[t](value) for t in types): - return [f"{path}: {value!r} is not of type {types}"] - if "const" in node and ( - value != node["const"] or type(value) is not type(node["const"]) - ): - errors.append(f"{path}: {value!r} is not {node['const']!r}") - if "oneOf" in node: - # branches only constrain; ``exact`` is the enclosing node's job - matches = [ - not check(value, branch, path, False) for branch in node["oneOf"] - ] - if matches.count(True) != 1: - errors.append(f"{path}: matches {matches.count(True)} of oneOf") - if isinstance(value, str): - if len(value) < node.get("minLength", 0): - errors.append(f"{path}: too short") - if len(value) > node.get("maxLength", len(value)): - errors.append(f"{path}: too long") - if "pattern" in node and not re.search(node["pattern"], value): - errors.append(f"{path}: {value!r} does not match {node['pattern']}") - if isinstance(value, list): - if len(value) < node.get("minItems", 0): - errors.append(f"{path}: too few items") - if node.get("uniqueItems") and len(set(map(json.dumps, value))) != len( - value - ): - errors.append(f"{path}: items are not unique") - if "items" in node: - for index, item in enumerate(value): - errors += check(item, node["items"], f"{path}[{index}]", exact) - if isinstance(value, dict): - for key in node.get("required", []): - if key not in value: - errors.append(f"{path}: missing {key}") - properties = node.get("properties", {}) - for key, item in value.items(): - if key in properties: - errors += check(item, properties[key], f"{path}.{key}", exact) - elif "additionalProperties" in node: - extra = node["additionalProperties"] - errors += check(item, extra, f"{path}.{key}", exact) - elif exact: - errors.append(f"{path}: undocumented property {key}") - return errors - - return check(instance, root, "$", exact) - - -def _schema_nodes(node, *, branches=True): - """``node`` and every schema below it; ``branches``: including oneOf's.""" - yield node - children = [ - *node.get("properties", {}).values(), - *node.get("$defs", {}).values(), - ] - for key in ("items", "additionalProperties"): - if isinstance(node.get(key), dict): - children.append(node[key]) - if branches: - children += node.get("oneOf", []) - for child in children: - yield from _schema_nodes(child, branches=branches) - def test_schema_uses_only_supported_keywords(): - for node in _schema_nodes(_schema()): + for node in schema_nodes(catalog_schema()): assert set(node) <= SUPPORTED_KEYWORDS, set(node) - SUPPORTED_KEYWORDS if "$ref" in node: # schema_errors merges a $ref's siblings into it assert set(node) <= {"$ref", "description"} @@ -151,13 +37,13 @@ def test_schema_requires_every_property_it_describes(): # additive fields may be optional to consumers, but skilldeck always # emits every field (null when empty), so the schema requires them all # (oneOf branches only add constraints to properties required above them) - for node in _schema_nodes(_schema(), branches=False): + for node in schema_nodes(catalog_schema(), branches=False): if "properties" in node: assert set(node["required"]) == set(node["properties"]) def test_schema_version_matches_the_code(): - schema = _schema() + schema = catalog_schema() assert schema["properties"]["schema_version"]["const"] == CATALOG_SCHEMA_VERSION assert f"schema_version {CATALOG_SCHEMA_VERSION}" in schema["title"] diff --git a/tests/test_lifecycle.py b/tests/test_lifecycle.py index 6027c77..9cca4fc 100644 --- a/tests/test_lifecycle.py +++ b/tests/test_lifecycle.py @@ -20,6 +20,7 @@ import pytest from click.testing import CliRunner +from _schema import schema_errors from skilldeck import __version__, registry from skilldeck.adapters import ADAPTERS, ALL_ADAPTERS from skilldeck.adapters import base as adapter_base @@ -27,7 +28,6 @@ from skilldeck.cli import cli from skilldeck.provenance import content_manifest from skilldeck.registry import discover_skills -from test_catalog import schema_errors _ROOT = Path(__file__).resolve().parent.parent _SCRIPT = _ROOT / "scripts" / "check_lifecycle.py" @@ -149,8 +149,26 @@ def test_parse_changelog_splits_sections_groups_and_entries(): "`other` goes too.", ] assert unreleased.entries("Deprecated") == ["`legacy-review`"] # title-cased - # [Unreleased] and the newest dated section by number, not file order - assert [s.version for s in check.recent_sections(sections)] == [None, "0.10.0"] + # [Unreleased], plus the dated sections the base does not have yet + recent = check.recent_sections(sections, {"0.3.0", "0.1.0"}) + assert [s.version for s in recent] == [None, "0.10.0"] + recent = check.recent_sections(sections, {"0.3.0", "0.10.0", "0.1.0"}) + assert [s.version for s in recent] == [None] + + +@pytest.mark.parametrize("day", ["2026-06-31", "2026-02-29", "2026-13-01"]) +def test_an_impossible_date_is_a_clean_error(day): + with pytest.raises(check.ChangelogError, match=f"`## \\[0.2.0\\] - {day}`"): + check.parse_changelog(f"## [0.2.0] - {day}\n", "CHANGELOG.md") + + +def test_main_reports_an_impossible_date(tmp_path, monkeypatch, capsys): + _write(tmp_path / "CHANGELOG.md", "## [Unreleased]\n\n## [0.2.0] - 2026-06-31\n") + monkeypatch.setattr(check, "ROOT", tmp_path) + assert check.main([]) == 1 + assert "error: CHANGELOG.md: `## [0.2.0] - 2026-06-31` has an invalid date" in ( + capsys.readouterr().err + ) def test_names_needs_every_term_in_backticks(): @@ -214,7 +232,7 @@ def test_release_bump_rule_needs_two_dated_releases(): # --- a synthetic repository ----------------------------------------------------- -def _git(repo, *args): +def _git(repo, *args, env=None): subprocess.run( ["git", "-C", str(repo), *args], check=True, @@ -225,6 +243,7 @@ def _git(repo, *args): "GIT_AUTHOR_EMAIL": "t@example.com", "GIT_COMMITTER_NAME": "t", "GIT_COMMITTER_EMAIL": "t@example.com", + **(env or {}), }, ) @@ -268,9 +287,13 @@ def write_version(repo, version): _write(repo / "pyproject.toml", f'[project]\nname = "x"\nversion = "{version}"\n') -def commit(repo, message="change"): +def commit(repo, message="change", date=None): + """Commit everything; ``date`` (YYYY-MM-DD) also dates a lightweight tag + made on it, as ``git for-each-ref``'s ``creatordate`` reads it.""" _git(repo, "add", "-A") - _git(repo, "commit", "-q", "-m", message) + when = f"{date}T12:00:00Z" if date else None + env = {"GIT_AUTHOR_DATE": when, "GIT_COMMITTER_DATE": when} if when else {} + _git(repo, "commit", "-q", "-m", message, env=env) @pytest.fixture @@ -326,33 +349,107 @@ def test_removing_a_skill_needs_a_removed_entry_and_a_deprecation(repo): ) assert "docs/lifecycle.md#removing-a-skill" in found[0] assert "without being deprecated first" in found[1] - assert "`### Security` entry naming `old-review`" in found[1] + assert "marks its `### Removed` entry `**Security:**`" in found[1] write_changelog(repo, "### Removed\n\n- `old-review`: use `other-review`.\n") assert [e for e in errors() if "deprecated first" not in e] == [] -def test_an_entry_in_the_wrong_group_or_section_does_not_count(repo): +def test_an_entry_in_the_wrong_group_does_not_count(repo): shutil.rmtree(repo / SKILLS / "old-review") write_changelog( repo, "### Changed\n\n- `old-review` is gone.\n", - "## [0.2.0] - 2026-02-01\n\n### Added\n\n- `old-review`\n\n" - "## [0.1.1] - 2026-01-15\n\n### Removed\n\n- `old-review`\n\n", + "## [0.2.0] - 2026-02-01\n\n### Added\n\n- `old-review`\n\n", ) assert any("add a `### Removed` entry" in e for e in errors()) +def test_a_published_section_does_not_count(repo): + # reviewer probe A: a Removed entry an earlier release already shipped + # (here, for dropping an agent) says nothing to the next release's users + _git(repo, "tag", "v0.1.0") + write_skill(repo, "old-review", "1.3.0", ("claude",), since="1.3.0", reason="R") + published = ( + "## [0.2.0] - 2026-06-01\n\n### Deprecated\n\n- `old-review`: obsolete.\n\n" + "### Removed\n\n- `old-review` no longer supports `codex`.\n\n" + ) + write_changelog(repo, released=published) + commit(repo, "release 0.2.0", "2026-06-01") + _git(repo, "tag", "v0.2.0") + shutil.rmtree(repo / SKILLS / "old-review") + assert [e.split(":")[0] for e in errors(datetime.date(2026, 12, 31))] == [ + "skill `old-review` was removed" + ] + + +def test_a_name_is_matched_whole(repo): + # reviewer probe G: a note for `old-review` is not one for `old-review-extra` + write_skill(repo, "old-review-extra") + commit(repo) + shutil.rmtree(repo / SKILLS / "old-review-extra") + write_changelog(repo, "### Removed\n\n- `old-review`\n") + assert any(e.startswith("skill `old-review-extra` was removed:") for e in errors()) + + def test_an_urgent_security_removal_skips_the_deprecation(repo): shutil.rmtree(repo / SKILLS / "old-review") write_changelog( repo, "### Security\n\n- `old-review` told agents to disable TLS checks.\n\n" - "### Removed\n\n- `old-review`, for the reason under Security.\n", + "### Removed\n\n- **Security:** `old-review`, for the reason under " + "Security.\n", ) assert errors() == [] +@pytest.mark.parametrize( + "notes", + [ + # reviewer probe D: a Security entry that merely mentions the skill + "### Security\n\n- `other-review` now flags SSRF, like `old-review`.\n\n" + "### Removed\n\n- `old-review`.\n", + # the mark alone, with no Security entry saying why + "### Removed\n\n- **Security:** `old-review`.\n", + # the mark on another entry + "### Security\n\n- `old-review` is harmful.\n\n" + "### Removed\n\n- `old-review`.\n- **Security:** `other-thing`.\n", + ], +) +def test_the_security_path_needs_the_marked_removal_and_a_security_entry(repo, notes): + shutil.rmtree(repo / SKILLS / "old-review") + write_changelog(repo, notes) + (found,) = errors() + assert "without being deprecated first" in found + + +def test_removing_a_skill_against_the_real_changelog(repo): + # the real [Unreleased] once filed Added entries naming skills under + # Security; removing one of them still takes the full path + text = (_ROOT / "CHANGELOG.md").read_text(encoding="utf-8") + write_skill(repo, "security-review") + _write(repo / "CHANGELOG.md", text) + commit(repo) + shutil.rmtree(repo / SKILLS / "security-review") + head = "## [Unreleased]\n\n### Removed\n\n- `security-review`.\n" + _write(repo / "CHANGELOG.md", text.replace("## [Unreleased]\n", head, 1)) + (found,) = errors() + assert found.startswith("skill `security-review` was removed without being") + + +def test_the_real_security_section_names_no_skill_or_adapter(): + unreleased = check.parse_changelog( + (_ROOT / "CHANGELOG.md").read_text(encoding="utf-8") + )[0] + named = {skill.name for skill in discover_skills()} | set(ALL_ADAPTERS) + assert [ + name + for name in sorted(named) + for entry in unreleased.entries("Security") + if check.names(entry, name) + ] == [] + + def test_removing_a_deprecated_skill_no_release_contains(repo): # never in a tagged release: nobody holds a release to give notice to write_skill(repo, "old-review", reason="Obsolete.", since="1.2.0") @@ -363,27 +460,29 @@ def test_removing_a_deprecated_skill_no_release_contains(repo): assert errors() == [] +DEPRECATED_IN_0_2_0 = ( + "## [0.2.0] - 2026-06-01\n\n### Deprecated\n\n" + "- `old-review`: obsolete; removal no earlier than 90 days from now.\n\n" +) + + @pytest.fixture def released_deprecation(repo): - """``old-review`` shipped deprecated in the tagged release 0.2.0.""" + """``old-review`` shipped deprecated in release 0.2.0, tagged 2026-06-01.""" write_skill(repo, "old-review", reason="Obsolete.", since="1.2.0") - deprecated = ( - "## [0.2.0] - 2026-06-01\n\n### Deprecated\n\n" - "- `old-review`: obsolete; removal no earlier than 90 days from now.\n\n" - ) - write_changelog(repo, released=deprecated) - commit(repo, "release 0.2.0") + write_changelog(repo, released=DEPRECATED_IN_0_2_0) + commit(repo, "release 0.2.0", "2026-06-01") _git(repo, "tag", "v0.2.0") shutil.rmtree(repo / SKILLS / "old-review") - write_changelog(repo, "### Removed\n\n- `old-review`\n", deprecated) + write_changelog(repo, "### Removed\n\n- `old-review`\n", DEPRECATED_IN_0_2_0) return repo def test_removal_waits_for_the_notice_period(released_deprecation): assert errors(datetime.date(2026, 8, 29)) == [ - "skill `old-review` was removed too early: its deprecation was published " - "in 0.2.0 on 2026-06-01, so it can be removed from 2026-08-30 (90 days' " - "notice; docs/lifecycle.md#removing-a-skill)" + "skill `old-review` was removed too early: v0.2.0 published its " + "deprecation on 2026-06-01, so it can be removed from 2026-08-30 (90 " + "days' notice; docs/lifecycle.md#removing-a-skill)" ] assert errors(datetime.date(2026, 8, 30)) == [] @@ -395,12 +494,67 @@ def test_from_1_0_the_notice_period_is_180_days(released_deprecation): assert errors(datetime.date(2026, 11, 28)) == [] -def test_the_notice_counts_only_from_a_published_release(released_deprecation): - # a dated section without its tag is prepared, not published +def test_the_notice_starts_when_the_release_is_tagged(repo): + # reviewer probe C: prepared (dated) 2026-01-01, but published 2026-03-25 + write_skill(repo, "old-review", reason="Obsolete.", since="1.2.0") + dated = DEPRECATED_IN_0_2_0.replace("2026-06-01", "2026-01-01") + write_changelog(repo, released=dated) + commit(repo, "release 0.2.0", "2026-03-25") + _git(repo, "tag", "v0.2.0") + shutil.rmtree(repo / SKILLS / "old-review") + write_changelog(repo, "### Removed\n\n- `old-review`\n", dated) + (found,) = errors(datetime.date(2026, 4, 2)) + assert "v0.2.0 published its deprecation on 2026-03-25" in found + assert errors(datetime.date(2026, 6, 23)) == [] + + +@pytest.mark.parametrize("retrofit", [False, True]) +def test_only_a_release_that_shipped_the_deprecation_counts(repo, retrofit): + # reviewer probe B: 0.2.0 shipped old-review undeprecated; the deprecation + # merged afterwards but no release carried it. Writing a Deprecated entry + # into the published 0.2.0 section later changes nothing. + released = "## [0.2.0] - 2026-01-01\n\n### Added\n\n- stuff\n\n" + write_changelog(repo, released=released) + commit(repo, "release 0.2.0", "2026-01-01") + _git(repo, "tag", "v0.2.0") + write_skill(repo, "old-review", "1.3.0", since="1.3.0", reason="Obsolete.") + write_changelog(repo, "### Deprecated\n\n- `old-review`\n", released) + commit(repo, "deprecate, unreleased") + shutil.rmtree(repo / SKILLS / "old-review") + if retrofit: + released += "### Deprecated\n\n- `old-review`\n\n" + write_changelog(repo, "### Removed\n\n- `old-review`\n", released) + (found,) = errors(datetime.date(2026, 12, 31)) + assert "no release published its deprecation" in found + + +def test_a_tag_off_main_is_not_a_release(repo): + _git(repo, "tag", "v0.1.0") # old-review is in a release + for branch in ("side", "main"): # the same deprecation on both branches + _git(repo, "checkout", "-q", "-B", branch) + write_skill(repo, "old-review", reason="Obsolete.", since="1.2.0") + write_changelog(repo, released=DEPRECATED_IN_0_2_0) + commit(repo, f"release 0.2.0 on {branch}", "2026-06-01") + if branch == "side": + _git(repo, "tag", "v0.2.0") # never reachable from main + _git(repo, "checkout", "-q", "v0.1.0") + shutil.rmtree(repo / SKILLS / "old-review") + write_changelog(repo, "### Removed\n\n- `old-review`\n", DEPRECATED_IN_0_2_0) + (found,) = errors(datetime.date(2026, 12, 31)) + assert "no release published its deprecation" in found + + +def test_without_release_tags_the_notice_rules_are_skipped_with_a_note( + released_deprecation, capsys +): _git(released_deprecation, "tag", "-d", "v0.2.0") - _git(released_deprecation, "tag", "v0.1.0", "HEAD") # the skill was released - (found,) = errors(datetime.date(2027, 1, 1)) - assert "no published release announces its deprecation" in found + assert check.main(["--base", "main"]) == 0 + captured = capsys.readouterr() + assert captured.err == f"note: {check.NO_TAGS_NOTE}\n" + assert "`git fetch --tags`" in check.NO_TAGS_NOTE + _git(released_deprecation, "tag", "v0.2.0") + check.main(["--base", "main"]) + assert "note:" not in capsys.readouterr().err # --- deprecating, dropping an agent, a major version, the catalog schema ------ @@ -496,7 +650,7 @@ def test_walkthrough_renaming_a_skill(repo): # 2. the release that publishes the deprecation released = notes.replace("### Added", "## [0.2.0] - 2026-06-01\n\n### Added") write_changelog(repo, released=released + "\n") - commit(repo, "release 0.2.0") + commit(repo, "release 0.2.0", "2026-06-01") _git(repo, "tag", "v0.2.0") # 3. at least 90 days later, the removal PR @@ -561,11 +715,17 @@ def test_walkthrough_removing_an_adapter(repo, monkeypatch, adapter): # the adapter's note, not one per skill write_skill(repo, "other-review", agents=("claude", "codex")) assert errors() == [ - f"adapter `{adapter}` was removed: add a `### Removed` entry naming " - f"`{adapter}` (in backticks) under `## [Unreleased]` in CHANGELOG.md, " - "listing the paths it installed to so users can delete what is left " - "(docs/lifecycle.md#removing-an-agent-or-format)" + f"adapter `{adapter}` was removed: add a `### Removed` entry of its own " + f"naming `{adapter}` (in backticks) and no skill under `## [Unreleased]` " + "in CHANGELOG.md, listing the paths it installed to so users can delete " + "what is left (docs/lifecycle.md#removing-an-agent-or-format)" ] + # reviewer probe E: an entry about one skill and the agent is not the + # note that the adapter itself is gone + write_changelog( + repo, f"### Removed\n\n- `other-review` no longer supports `{adapter}`.\n" + ) + assert len(errors()) == 1 write_changelog(repo, f"### Removed\n\n- The `{adapter}` adapter.\n") assert errors() == [] @@ -573,9 +733,10 @@ def test_walkthrough_removing_an_adapter(repo, monkeypatch, adapter): def test_main_reports_what_is_missing_and_links_the_policy(repo, capsys): shutil.rmtree(repo / SKILLS / "old-review") assert check.main(["--base", "main"]) == 1 - err = capsys.readouterr().err - assert err.startswith("error: skill `old-review` was removed") - assert "See docs/lifecycle.md" in err + err = capsys.readouterr().err.splitlines() + assert err[0] == f"note: {check.NO_TAGS_NOTE}" + assert err[1].startswith("error: skill `old-review` was removed") + assert "See docs/lifecycle.md" in err[-2] # --- what happens to installed copies (docs/lifecycle.md says so) ------------- diff --git a/tests/test_prepare_release.py b/tests/test_prepare_release.py index e711e78..8b71e11 100644 --- a/tests/test_prepare_release.py +++ b/tests/test_prepare_release.py @@ -165,6 +165,19 @@ def test_main_refuses_existing_changelog_section_without_bumping(tree, monkeypat assert _snapshot(tree) == before +def test_main_reports_an_impossible_changelog_date_cleanly(tree, monkeypatch): + run, calls = _fake_run(0) + monkeypatch.setattr(prep.subprocess, "run", run) + (tree / "CHANGELOG.md").write_text( + CHANGELOG.replace("2026-06-27", "2026-06-31"), encoding="utf-8" + ) + before = _snapshot(tree) + with pytest.raises(SystemExit, match=r"2026-06-31` has an invalid date"): + prep.main(["0.4.0"]) + assert _snapshot(tree) == before + assert calls == [] + + @pytest.mark.parametrize("group", ["Removed", "Deprecated"]) def test_main_refuses_a_patch_release_that_removes_or_deprecates( tree, monkeypatch, group