From 34d95f70cd23fe71662c5a6c3d8e61460c5f29da Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 09:24:25 +0000 Subject: [PATCH 1/2] Publish an agent compatibility matrix backed by adapter contract tests Add docs/compatibility.md, linked from the README and docs/adapters.md. It covers every adapter: the five native ones and the copilot-prompt, cursor-rule and kiro-steering legacy adapters. Each row gives a status (tested, supported or experimental, defined on the page), the project and global paths, the environment variables that move them, how the skill is invoked, the minimum agent version, and the date, agent version and vendor sources it was checked against. The research behind it was done on 2026-09-23, and whatever it couldn't confirm is marked unverified. The page also sets out how skilldeck responds when a vendor deprecates or moves a skill location or format. tests/fixtures/adapter-contracts/ pins each adapter's exact rendered file (stamp included) for one synthetic skill, its project and global paths, its --scope global error if it is project-only, and its behaviour with each config-directory variable set to an absolute path, left empty or set to a relative path. tests/test_adapter_contracts.py installs the skill with every adapter into temporary directories and compares the results byte for byte. The fixtures are marked -text in .gitattributes so Windows checkouts keep them exact. The matrix embeds a sha256 digest of the fixtures, and the changelog must mention its first 12 characters, so a format change fails CI until the matrix and changelog are updated. The error for an unsupported scope now names what works instead. For example, "Use --scope project, or --agent cursor (Agent Skills), which supports --scope global". Closes #79. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01HtiCGzpikMrkDYBkfQG5CX --- .gitattributes | 3 + CHANGELOG.md | 21 ++ CLAUDE.md | 11 +- README.md | 4 +- docs/adapters.md | 4 + docs/compatibility.md | 249 ++++++++++++++++++ src/skilldeck/adapters/base.py | 27 +- src/skilldeck/adapters/legacy.py | 14 +- .../adapter-contracts/claude/SKILL.md | 14 + .../fixtures/adapter-contracts/codex/SKILL.md | 14 + .../fixtures/adapter-contracts/contracts.json | 126 +++++++++ .../copilot-prompt/contract-demo.prompt.md | 14 + .../adapter-contracts/copilot/SKILL.md | 14 + .../cursor-rule/contract-demo.mdc | 13 + .../adapter-contracts/cursor/SKILL.md | 14 + .../kiro-steering/contract-demo.md | 12 + .../fixtures/adapter-contracts/kiro/SKILL.md | 14 + .../skill/contract-demo/meta.yaml | 10 + .../skill/contract-demo/skill.md | 7 + tests/test_adapter_contracts.py | 249 ++++++++++++++++++ tests/test_adapters.py | 9 + tests/test_cli.py | 2 + 22 files changed, 836 insertions(+), 9 deletions(-) create mode 100644 .gitattributes create mode 100644 docs/compatibility.md create mode 100644 tests/fixtures/adapter-contracts/claude/SKILL.md create mode 100644 tests/fixtures/adapter-contracts/codex/SKILL.md create mode 100644 tests/fixtures/adapter-contracts/contracts.json create mode 100644 tests/fixtures/adapter-contracts/copilot-prompt/contract-demo.prompt.md create mode 100644 tests/fixtures/adapter-contracts/copilot/SKILL.md create mode 100644 tests/fixtures/adapter-contracts/cursor-rule/contract-demo.mdc create mode 100644 tests/fixtures/adapter-contracts/cursor/SKILL.md create mode 100644 tests/fixtures/adapter-contracts/kiro-steering/contract-demo.md create mode 100644 tests/fixtures/adapter-contracts/kiro/SKILL.md create mode 100644 tests/fixtures/adapter-contracts/skill/contract-demo/meta.yaml create mode 100644 tests/fixtures/adapter-contracts/skill/contract-demo/skill.md create mode 100644 tests/test_adapter_contracts.py diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..556050b --- /dev/null +++ b/.gitattributes @@ -0,0 +1,3 @@ +# The adapter contract tests compare these files byte for byte (and hash them +# for docs/compatibility.md), so never convert their line endings on checkout. +tests/fixtures/adapter-contracts/** -text diff --git a/CHANGELOG.md b/CHANGELOG.md index 7df8fde..e01ae79 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -141,6 +141,9 @@ All notable changes to this project are documented here. The format is based on ### Changed +- Asking an adapter for a scope it doesn't support now tells you what works + instead (#79). For example, `--agent cursor-rule --scope global` names + `--scope project`, or `--agent cursor`, which supports `--scope global`. - Five review skills cover defect classes their checklists missed, each cited to a fetched source (#104): - `ci-workflow-review` 0.4.0: newline injection through writes to @@ -498,6 +501,24 @@ All notable changes to this project are documented here. The format is based on ### Added +- 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: + - a status (tested, supported or experimental; the page defines each) + - project and global paths, and the environment variables that move them + - how you invoke the skill in that agent + - the minimum agent version + - the date, agent version and vendor sources it was checked against + Whatever couldn't be checked is marked unverified. The page also says how + skilldeck responds when a vendor deprecates or moves a skill location or + format. Adapter contract fixtures (`tests/fixtures/adapter-contracts/`, + contract `sha256:b3cbf84dafa6`) pin each adapter's exact rendered file, + stamp included, for one synthetic skill. They also pin its project and + global paths, and its behaviour with each config-directory variable set to + an absolute path, left empty, or set to a relative path. CI installs with + every adapter and compares the results byte for byte. A test fails when + the fixtures change unless the matrix's `adapter-contract` digest and this + changelog are updated too. - `frontend-security-review` skill (0.1.0) (#105) — reviews browser-side changes (React, Vue, Angular, Svelte, plain JavaScript and HTML templates, CSP and header config): framework escape hatches (`dangerouslySetInnerHTML`, diff --git a/CLAUDE.md b/CLAUDE.md index d5e49ea..8634d42 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -71,7 +71,11 @@ by `--agent all`); `skilldeck migrate` moves old-format installs to `SKILL.md`. `tests/test_eval_fixtures.py`, and unit-tests the scorer (`tests/test_eval_scoring.py`). See `evals/README.md`. New/changed skills should be run through them. -- `docs/` — `authoring-skills.md`, `adapters.md`, `releasing.md` +- `docs/` — `authoring-skills.md`, `adapters.md`, `compatibility.md` (the + public agent compatibility matrix), `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` - `.claude-plugin/marketplace.json` + `claude-plugin/` — the Claude Code plugin marketplace tree, **generated** by `scripts/build_plugin.py` from the canonical skills; regenerate after changing skills or the project version (a @@ -97,6 +101,11 @@ by `--agent all`); `skilldeck migrate` moves old-format installs to `SKILL.md`. output. - A skill's `meta.yaml` `name` must match its directory name; all metadata fields are required and validated by the registry. +- Any change to an adapter's output format, paths or env handling must update + its contract fixtures, the matrix rows and `adapter-contract` digest in + `docs/compatibility.md`, and CHANGELOG (the contract tests enforce all three; + see `docs/compatibility.md#contract-tests`). Only state vendor behaviour a + primary source confirms; mark the rest *unverified*. - New skills follow the structural template (enforced by `tests/test_skill_structure.py`), ground their checklists in **fetched** authoritative sources (OWASP/CIS/vendor docs) cited in the skill body, and diff --git a/README.md b/README.md index f5c9e75..563b3fc 100644 --- a/README.md +++ b/README.md @@ -28,7 +28,9 @@ Every agent gets the same [Agent Skills](https://agentskills.io/specification) versions too old for skills, the earlier formats remain available as the `copilot-prompt`, `cursor-rule` and `kiro-steering` adapters. See [docs/adapters.md](docs/adapters.md) for each agent's locations and minimum -version. +version, and the [compatibility matrix](docs/compatibility.md) for how each +adapter is invoked, which agent versions it was checked against, and how +well. ## Claude Code: install as a plugin (no Python needed) diff --git a/docs/adapters.md b/docs/adapters.md index 17ea6f0..b32834b 100644 --- a/docs/adapters.md +++ b/docs/adapters.md @@ -11,6 +11,8 @@ adapter** (`ADAPTERS`, named after the agent) writes that format into the agent's own skills folder, and all five render byte-identical files; only the folders differ. The formats skilldeck used before remain available as opt-in [legacy adapters](#legacy-adapters) for agent versions that predate skills. +The [compatibility matrix](compatibility.md) sums up each adapter's status, +how it's invoked, and the agent versions it was checked against. ## Install locations @@ -329,6 +331,8 @@ land wherever it points. should move installs out of it. 3. Add the agent name to the `supported-agents` list of any skill it should apply to. +4. Add the adapter's contract to `tests/fixtures/adapter-contracts/` and its + row to the [compatibility matrix](compatibility.md#contract-tests). The base class handles `install`/`uninstall` (including the stamp checks, symlink handling and atomic writes described above), directory creation, and diff --git a/docs/compatibility.md b/docs/compatibility.md new file mode 100644 index 0000000..35652cf --- /dev/null +++ b/docs/compatibility.md @@ -0,0 +1,249 @@ +# Agent compatibility + + + +This page lists what skilldeck installs for each agent and where it goes. It +also covers how you then use a skill in that agent, the agent version it needs, +and how well each entry has been checked. [Adapters](adapters.md) explains the +formats, the environment variables and duplicate handling in more detail. + +Each row says when it was last checked, and against which agent version and +sources. Every row was last checked on 2026-09-23. Anything that couldn't be +checked against a vendor source is marked *unverified*. + +## Status + +- **tested**: the agent was run at the listed version and seen to load a + skill installed at these locations. +- **supported**: the vendor's own source code, shipped binary or documentation + at the listed version confirms the locations, format and variables. The + agent itself was not run against them. +- **experimental**: skilldeck installs as described, but a vendor source + contradicts part of the row, or the format is deprecated or doesn't load on + some of the agent's surfaces. Read the notes before relying on it. + +Status covers the agent side. Every adapter's own side is tested the same way, +whatever its status: CI checks the exact bytes, paths and stamp it writes on +every pull request, on Linux, macOS and Windows (see +[Contract tests](#contract-tests)). + +## Matrix + +`` is the skill's name. `~` is your home directory. + +| Adapter (`--agent`) | Status | `--scope project` | `--scope global` | Moved by (global only) | Minimum agent version | Last checked | +|---|---|---|---|---|---|---| +| `claude` | tested | `.claude/skills//SKILL.md` | `~/.claude/skills//SKILL.md` | `CLAUDE_CONFIG_DIR`, to `$CLAUDE_CONFIG_DIR/skills`; an empty value is refused | Claude Code 2.0.20 (project skills fixed in 2.0.24) | 2026-09-23, Claude Code 2.1.281 | +| `codex` | supported | `.agents/skills//SKILL.md` | `~/.agents/skills//SKILL.md` | nothing (`CODEX_HOME` doesn't move it) | Codex 0.95.0 | 2026-09-23, Codex source at `rust-v0.156.1` | +| `copilot` | supported | `.github/skills//SKILL.md` | `~/.copilot/skills//SKILL.md` | `COPILOT_HOME`, to `$COPILOT_HOME/skills` (Copilot CLI only) | VS Code 1.109; Copilot CLI 0.0.371; the cloud agent and code review have no version | 2026-09-23, GitHub docs, VS Code source, Copilot CLI changelog up to 1.0.88 | +| `cursor` | supported | `.cursor/skills//SKILL.md` | `~/.cursor/skills//SKILL.md` | nothing | *unverified* | 2026-09-23, the skills loader in `@cursor/sdk` 1.0.32 (the desktop app was not inspected) | +| `kiro` | supported | `.kiro/skills//SKILL.md` | `~/.kiro/skills//SKILL.md` | `KIRO_HOME`, to `$KIRO_HOME/skills` (Kiro CLI; the IDE is *unverified*) | *unverified* | 2026-09-23, Kiro CLI 2.24.0 (binary and embedded docs); the IDE from Kiro's docs only | +| `copilot-prompt` | experimental | `.github/prompts/.prompt.md` | not supported | n/a | *unverified* | 2026-09-23, VS Code source and docs | +| `cursor-rule` | supported | `.cursor/rules/.mdc` | not supported | n/a | *unverified* | 2026-09-23, the rules loader in `@cursor/sdk` 1.0.32 | +| `kiro-steering` | experimental | `.kiro/steering/.md` | `~/.kiro/steering/.md` | `KIRO_HOME`, to `$KIRO_HOME/steering` (Kiro CLI; the IDE is *unverified*) | *unverified* | 2026-09-23, Kiro CLI 2.24.0 embedded docs; Kiro's docs mirror | + +The first five are the native adapters that `--agent all` selects. The last +three are opt-in [legacy adapters](adapters.md#legacy-adapters) for agent +versions that predate skills. + +When a variable in the "Moved by" column is set, skilldeck needs an absolute +path in it. It refuses a relative one, because the agent would resolve it against whatever +directory it was started in. `COPILOT_HOME` and `KIRO_HOME` count as unset +when empty. See [Environment variables](adapters.md#environment-variables). + +Asking `copilot-prompt` or `cursor-rule` for `--scope global` fails, and the +error names what works instead: + +```text +error: cursor-rule does not support --scope global: it has no stable file location for that scope. Use --scope project, or --agent cursor (Agent Skills), which supports --scope global +``` + +## Using a skill in each agent + +### `claude`: Claude Code + +- **Invoked** by typing `/` (the folder name). Claude also uses a skill + on its own when the task matches its `description`. Project skills load + from the directory Claude Code starts in and every parent up to the + repository root. +- **Evidence**: `code.claude.com/docs/en/skills.md` lines 120–121 and 138; + `env-vars.md` line 403 and `claude-directory.md` line 1435 + (`CLAUDE_CONFIG_DIR`); `anthropics/claude-code@d78be94` `CHANGELOG.md` lines + 6648 (2.0.20) and 6626 (2.0.24). Claude Code 2.1.281 was run: project and + personal skills loaded, `CLAUDE_CONFIG_DIR` moved personal skills to + `$CLAUDE_CONFIG_DIR/skills`, and `.agents/skills` was not read. +- **Unverified**: 2.0.20 as the first version rests on the changelog alone, + as no older build was run. The behaviour of an empty `CLAUDE_CONFIG_DIR` was + observed in 2.1.281 only and is undocumented. + +### `codex`: OpenAI Codex + +- **Invoked** by mentioning `$` in a prompt or picking the skill from + `/skills` in the TUI. Codex also uses a skill when the task clearly + matches its description. A plain `$name` resolves only when exactly one + enabled skill has that name, so install each skill at one scope only. +- **Evidence**: `openai/codex@17cd2834`, whose skill-loading files match + tag `rust-v0.156.1`: `codex-rs/ext/skills/src/host_roots.rs` lines 95–108 and + 142–154, `codex-rs/skills/src/mentions.rs` line 41, + `codex-rs/skills/src/selection.rs` lines 188–190, + `codex-rs/ext/skills/src/catalog_prompt.rs` lines 3–8, and commits + `39a6a84097` (`rust-v0.94.0`) and `e24058b7a8` (`rust-v0.95.0`). +- **Unverified**: Codex's own skills documentation couldn't be fetched, and + Codex was not run. The IDE extension and Codex cloud were not checked. + +### `copilot`: GitHub Copilot + +- **Invoked** automatically when the task matches the skill's description, + in VS Code, the Copilot CLI, the cloud agent and code review. You can also + type `/` in VS Code chat or the Copilot CLI. A skill added while a + Copilot CLI session is running needs `/skills reload`. +- **Evidence**: `github/docs@7922319f` + `content/copilot/concepts/agents/about-agent-skills.md` lines 27–28, + `cli-command-reference.md` lines 1141–1155 and `cli-config-dir-reference.md` + lines 378–386; `microsoft/vscode@5dfa2a72` `promptFileLocations.ts` lines + 172–179; `microsoft/vscode-docs@44133f07` `release-notes/v1_109.md` lines + 449 and 455; `github/copilot-cli@57dd2440` `changelog.md` line 2886 + (0.0.371). +- **Unverified**: how VS Code's Agent Host sessions find skills (the Copilot + SDK they use is closed source), including whether they honour + `COPILOT_HOME`. Also unchecked: the skill folders Visual Studio and + JetBrains use, and how the Copilot CLI treats an empty `COPILOT_HOME`. + Copilot was not run. + +### `cursor`: Cursor + +- **Invoked**: Cursor offers each skill to the agent with its description, + and the agent decides when to use it. That choice is made on Cursor's + servers, so it is *unverified*. Cursor documents `/` only for skills + installed from a plugin; for skills in `.cursor/skills` it is *unverified*. +- **Evidence**: npm `@cursor/sdk` 1.0.32 `dist/esm/34.js` (the bundled skills + loader's folder table, byte offset 552915); `cursor/cookbook@6733ef81` + `sdk/dag-task-runner/README.md` lines 146–152. +- **Unverified**: Cursor's docs and changelog couldn't be reached, so the + first release with skills is unknown. Nobody checked that the desktop app + and the `cursor-agent` CLI use the SDK's loader unchanged. Cursor was not + run. + +### `kiro`: Kiro + +- **Invoked** automatically: Kiro reads each skill's name and description + when a chat session starts, and loads the whole skill when a request + matches. You can also type `/`, and any text after it is passed + along. `/context show` lists the loaded skills. In Kiro CLI, a prompt file + `.kiro/prompts/.md` of the same name takes precedence over the skill. +- **Evidence**: the Kiro CLI 2.24.0 binary (its default + `skill://.kiro/skills/*/SKILL.md` resource) and its embedded docs + (`docs/features/skills.md`, and `docs/commands/chat.md` for `KIRO_HOME`); + `kirodotdev/KiroCrew@f1f891b` `docs/reference/kiro-cli/skills.md`, a mirror + of `kiro.dev/docs/skills`, which covers the IDE and CLI together. +- **Unverified**: the live kiro.dev docs and the IDE couldn't be reached, so + the first release with skills is unknown, as is whether the IDE honours + `KIRO_HOME`. Nobody saw a skill load at runtime, because Kiro CLI chat + needs a login. + +### `copilot-prompt`: Copilot prompt files (legacy) + +- **Invoked** by hand only: `/` in chat, **Chat: Run Prompt**, or the + editor's play button. `agent: agent` makes it run in agent mode, where it + can run `git diff`. +- **Experimental** because prompt files load only in VS Code's local agent, + Visual Studio and JetBrains (preview). The Copilot CLI and GitHub.com don't + load them, and VS Code has deprecated them for its Agent Host sessions, + which don't load them either. Use `copilot` where you can. +- **Evidence**: `microsoft/vscode@5dfa2a72` `chatWidget.ts` lines 4127–4145 (a + prompt's `agent` sets the mode) and `promptFileLocations.ts`; + `microsoft/vscode-docs@44133f07` + `docs/agent-customization/prompt-files.md` line 28 (not loaded by the Agent + Host); GitHub's Copilot customization cheat sheet in `github/docs@7922319f` + (which surfaces load prompt files). +- **Unverified**: the minimum VS Code version, and whether VS Code now uses + the Agent Host by default. + +### `cursor-rule`: Cursor rules (legacy) + +- **Invoked** when the agent asks for it: a rule with a `description` and + `alwaysApply: false` is pulled in when the description matches the task. +- **Evidence**: npm `@cursor/sdk` 1.0.32 `dist/esm/34.js`, which loads + `.cursor/rules/**/*.mdc` and reads `.mdc` frontmatter one line at a time + (byte offset 565213). Running that reader showed that a folded description + is cut off, which is why this adapter writes it on one line. +- **Unverified**: the minimum Cursor version, and whether the desktop app's + `.mdc` reader matches the SDK's. No vendor source deprecates `.mdc` rules, + but none rules it out either. + +### `kiro-steering`: Kiro steering files (legacy) + +- **Invoked** by hand: `#` in the Kiro IDE, or `/context add` in Kiro + CLI. +- **Experimental** because Kiro's own sources disagree about the CLI. The + docs embedded in Kiro CLI 2.24.0 say an `inclusion: manual` file loads only + on request. Kiro's mirror of the current steering page says Kiro CLI + ignores inclusion modes and loads every steering file. Kiro CLI before + 2.19.0 is also reported to load them all. Where that happens, every review + prompt is in every session, so prefer `kiro`. +- **Evidence**: Kiro CLI 2.24.0 embedded `docs/features/steering-files.md`, + and the binary string "excluding steering file from context (non-always + inclusion mode)"; `kirodotdev/KiroCrew@f1f891b` `steering.md` lines 38–42. +- **Unverified**: which Kiro CLI engine or version the "every steering file + loads" statement applies to, and the 2.19.0 report. + +The full list of sources, with paths and line numbers, is in +[Adapters: Sources](adapters.md#sources). + +## Contract tests + +`tests/fixtures/adapter-contracts/` pins each adapter's side of this table: + +- `skill/contract-demo/` is a small synthetic skill. Its description needs + YAML quoting and folding, and its body has non-ASCII text. +- `contracts.json` gives each adapter's project and global paths, the text + its `--scope global` error must contain if it is project-only, and its + expected frontmatter. It also says where a global install goes when each + environment variable is set to an absolute path, left empty, or set to a + relative path. +- `/` holds the exact file the adapter installs, stamp included. + +`tests/test_adapter_contracts.py` installs the skill with every adapter into +temporary directories and checks the result against these fixtures byte for +byte. The fixtures are committed with LF line endings (`.gitattributes`), +so the comparison is exact on Windows too. + +The `adapter-contract` comment at the top of this page is a SHA-256 digest of +the fixture directory. When an adapter's format or location changes, the +fixtures must change, and then the test fails until this page and +`CHANGELOG.md` are updated: + +1. Regenerate the expected files with + `SKILLDECK_UPDATE_CONTRACTS=1 uv run --locked --extra dev pytest tests/test_adapter_contracts.py`, + and review the diff. +2. Update the affected rows above: status, paths, versions, and the date + and agent version they were checked against. +3. Replace the `adapter-contract` line with the one the test prints. +4. Add a CHANGELOG entry under `[Unreleased]` that mentions the first 12 + characters of the digest (`sha256:<12 hex>`). The test checks for it. + +## When a vendor changes a location or format + +Agents move skill folders, rename variables and retire formats. skilldeck +handles a change like this: + +1. **Confirm it** in a primary source: the vendor's source code at a release + tag, a shipped binary, or the vendor's docs. A blog post, forum thread or + issue title is only a lead. +2. **Deprecated but still loading**: mark the row *experimental*, and note + the deprecation and the agent version that announced it. Installs keep + working, so there is no need for an immediate release. +3. **Moved or replaced**: point the native adapter at the new location or + format. While any agent version still in use reads the old one, keep it as + an opt-in legacy adapter. Add it to `MIGRATIONS` so `skilldeck migrate` + moves existing installs. If the vendor removes the old format outright, + keep it only as a migration source, not an install target, as was done + when Codex dropped custom prompts in 0.118.0. +4. **Update the contract**: regenerate the fixtures, and update this page and + the CHANGELOG ([above](#contract-tests)). Mark the entry **Breaking:** if + existing installs have to move. +5. **Urgent**: if a current agent release stops loading what skilldeck + installs, skills disappear from that agent without any error. Release the + 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`. diff --git a/src/skilldeck/adapters/base.py b/src/skilldeck/adapters/base.py index 6acb990..4a990ae 100644 --- a/src/skilldeck/adapters/base.py +++ b/src/skilldeck/adapters/base.py @@ -152,13 +152,28 @@ def scopes(self) -> tuple[Scope, ...]: found.append(Scope.GLOBAL) return tuple(found) + def scope_alternative(self, scope: Scope) -> str | None: + """Another ``--agent`` that installs at ``scope`` when this one can't, + worded for :meth:`check_scope`'s error; None if there is none.""" + return None + def check_scope(self, scope: Scope) -> None: - """Raise :class:`SkillError` if this agent cannot install at ``scope``.""" - if scope not in self.scopes: - raise SkillError( - f"{self.name} does not support --scope {scope.value}: it has no " - "stable file location for that scope" - ) + """Raise :class:`SkillError` if this agent cannot install at ``scope``. + + The message names what does work: this adapter's other scope, and + any other adapter for the same agent that has ``scope``. + """ + if scope in self.scopes: + return + options = [f"--scope {other.value}" for other in self.scopes] + alternative = self.scope_alternative(scope) + if alternative: + options.append(alternative) + hint = f". Use {', or '.join(options)}" if options else "" + raise SkillError( + f"{self.name} does not support --scope {scope.value}: it has no " + f"stable file location for that scope{hint}" + ) def root(self, scope: Scope, project_root: Path | None = None) -> Path: """The directory this adapter installs into at ``scope``. diff --git a/src/skilldeck/adapters/legacy.py b/src/skilldeck/adapters/legacy.py index ec66919..c449167 100644 --- a/src/skilldeck/adapters/legacy.py +++ b/src/skilldeck/adapters/legacy.py @@ -13,7 +13,7 @@ from pathlib import Path from ..registry import Skill, SkillError -from ..targets import UserDir +from ..targets import Scope, UserDir from .base import Adapter, yaml_frontmatter @@ -43,6 +43,18 @@ def entry(self, skill: Skill) -> Path: def supports(self, skill: Skill) -> bool: return self.agent in skill.supported_agents + def scope_alternative(self, scope: Scope) -> str | None: + # imported here: the adapter registry imports this module + from . import ADAPTERS + + native = ADAPTERS.get(self.agent) + if native is None or scope not in native.scopes: + return None + return ( + f"--agent {native.name} (Agent Skills), " + f"which supports --scope {scope.value}" + ) + class CopilotPromptAdapter(LegacyAdapter): """VS Code prompt files: ``.github/prompts/.prompt.md``, run with diff --git a/tests/fixtures/adapter-contracts/claude/SKILL.md b/tests/fixtures/adapter-contracts/claude/SKILL.md new file mode 100644 index 0000000..5ae88d0 --- /dev/null +++ b/tests/fixtures/adapter-contracts/claude/SKILL.md @@ -0,0 +1,14 @@ +--- +name: contract-demo +description: 'Contract fixture: a synthetic review skill that pins every adapter''s + rendered bytes, long enough that YAML folds it onto a second line.' +--- + +# Contract demo + +A synthetic skill for the adapter contract tests — it is never installed for +real. Non-ASCII text (naïve café, 検査) must reach every agent as UTF-8. + +1. Run `git diff` and read the changed files. +2. Report each finding with its file and line. + diff --git a/tests/fixtures/adapter-contracts/codex/SKILL.md b/tests/fixtures/adapter-contracts/codex/SKILL.md new file mode 100644 index 0000000..5ae88d0 --- /dev/null +++ b/tests/fixtures/adapter-contracts/codex/SKILL.md @@ -0,0 +1,14 @@ +--- +name: contract-demo +description: 'Contract fixture: a synthetic review skill that pins every adapter''s + rendered bytes, long enough that YAML folds it onto a second line.' +--- + +# Contract demo + +A synthetic skill for the adapter contract tests — it is never installed for +real. Non-ASCII text (naïve café, 検査) must reach every agent as UTF-8. + +1. Run `git diff` and read the changed files. +2. Report each finding with its file and line. + diff --git a/tests/fixtures/adapter-contracts/contracts.json b/tests/fixtures/adapter-contracts/contracts.json new file mode 100644 index 0000000..e86a015 --- /dev/null +++ b/tests/fixtures/adapter-contracts/contracts.json @@ -0,0 +1,126 @@ +{ + "_about": [ + "The adapter contract: what installing skill/contract-demo writes for each adapter, and where.", + "project: path under the project root. global: path under ~, or null if the adapter is project-only,", + "in which case global_error lists text the --scope global error must contain.", + "env: per variable, where a global install goes when it is set to an absolute path ($VAR is its value),", + "to an empty string, or to a relative path; \"error\" means skilldeck refuses.", + "frontmatter: the rendered file's frontmatter, parsed as YAML.", + "The expected file is /, byte for byte, stamp included.", + "Changing anything here changes the digest in docs/compatibility.md; see that page." + ], + "skill": "contract-demo", + "adapters": { + "claude": { + "project": ".claude/skills/contract-demo/SKILL.md", + "global": "~/.claude/skills/contract-demo/SKILL.md", + "env": { + "CLAUDE_CONFIG_DIR": { + "absolute": "$CLAUDE_CONFIG_DIR/skills/contract-demo/SKILL.md", + "empty": "error", + "relative": "error" + } + }, + "frontmatter": { + "name": "contract-demo", + "description": "Contract fixture: a synthetic review skill that pins every adapter's rendered bytes, long enough that YAML folds it onto a second line." + } + }, + "codex": { + "project": ".agents/skills/contract-demo/SKILL.md", + "global": "~/.agents/skills/contract-demo/SKILL.md", + "env": { + "CODEX_HOME": { + "absolute": "~/.agents/skills/contract-demo/SKILL.md", + "empty": "~/.agents/skills/contract-demo/SKILL.md", + "relative": "~/.agents/skills/contract-demo/SKILL.md" + } + }, + "frontmatter": { + "name": "contract-demo", + "description": "Contract fixture: a synthetic review skill that pins every adapter's rendered bytes, long enough that YAML folds it onto a second line." + } + }, + "copilot": { + "project": ".github/skills/contract-demo/SKILL.md", + "global": "~/.copilot/skills/contract-demo/SKILL.md", + "env": { + "COPILOT_HOME": { + "absolute": "$COPILOT_HOME/skills/contract-demo/SKILL.md", + "empty": "~/.copilot/skills/contract-demo/SKILL.md", + "relative": "error" + } + }, + "frontmatter": { + "name": "contract-demo", + "description": "Contract fixture: a synthetic review skill that pins every adapter's rendered bytes, long enough that YAML folds it onto a second line." + } + }, + "cursor": { + "project": ".cursor/skills/contract-demo/SKILL.md", + "global": "~/.cursor/skills/contract-demo/SKILL.md", + "env": {}, + "frontmatter": { + "name": "contract-demo", + "description": "Contract fixture: a synthetic review skill that pins every adapter's rendered bytes, long enough that YAML folds it onto a second line." + } + }, + "kiro": { + "project": ".kiro/skills/contract-demo/SKILL.md", + "global": "~/.kiro/skills/contract-demo/SKILL.md", + "env": { + "KIRO_HOME": { + "absolute": "$KIRO_HOME/skills/contract-demo/SKILL.md", + "empty": "~/.kiro/skills/contract-demo/SKILL.md", + "relative": "error" + } + }, + "frontmatter": { + "name": "contract-demo", + "description": "Contract fixture: a synthetic review skill that pins every adapter's rendered bytes, long enough that YAML folds it onto a second line." + } + }, + "copilot-prompt": { + "project": ".github/prompts/contract-demo.prompt.md", + "global": null, + "global_error": [ + "copilot-prompt does not support --scope global", + "--scope project", + "--agent copilot" + ], + "env": {}, + "frontmatter": { + "description": "Contract fixture: a synthetic review skill that pins every adapter's rendered bytes, long enough that YAML folds it onto a second line.", + "agent": "agent" + } + }, + "cursor-rule": { + "project": ".cursor/rules/contract-demo.mdc", + "global": null, + "global_error": [ + "cursor-rule does not support --scope global", + "--scope project", + "--agent cursor" + ], + "env": {}, + "frontmatter": { + "description": "Contract fixture: a synthetic review skill that pins every adapter's rendered bytes, long enough that YAML folds it onto a second line.", + "alwaysApply": false + } + }, + "kiro-steering": { + "project": ".kiro/steering/contract-demo.md", + "global": "~/.kiro/steering/contract-demo.md", + "env": { + "KIRO_HOME": { + "absolute": "$KIRO_HOME/steering/contract-demo.md", + "empty": "~/.kiro/steering/contract-demo.md", + "relative": "error" + } + }, + "frontmatter": { + "inclusion": "manual" + } + } + } +} diff --git a/tests/fixtures/adapter-contracts/copilot-prompt/contract-demo.prompt.md b/tests/fixtures/adapter-contracts/copilot-prompt/contract-demo.prompt.md new file mode 100644 index 0000000..3c66f59 --- /dev/null +++ b/tests/fixtures/adapter-contracts/copilot-prompt/contract-demo.prompt.md @@ -0,0 +1,14 @@ +--- +description: 'Contract fixture: a synthetic review skill that pins every adapter''s + rendered bytes, long enough that YAML folds it onto a second line.' +agent: agent +--- + +# Contract demo + +A synthetic skill for the adapter contract tests — it is never installed for +real. Non-ASCII text (naïve café, 検査) must reach every agent as UTF-8. + +1. Run `git diff` and read the changed files. +2. Report each finding with its file and line. + diff --git a/tests/fixtures/adapter-contracts/copilot/SKILL.md b/tests/fixtures/adapter-contracts/copilot/SKILL.md new file mode 100644 index 0000000..5ae88d0 --- /dev/null +++ b/tests/fixtures/adapter-contracts/copilot/SKILL.md @@ -0,0 +1,14 @@ +--- +name: contract-demo +description: 'Contract fixture: a synthetic review skill that pins every adapter''s + rendered bytes, long enough that YAML folds it onto a second line.' +--- + +# Contract demo + +A synthetic skill for the adapter contract tests — it is never installed for +real. Non-ASCII text (naïve café, 検査) must reach every agent as UTF-8. + +1. Run `git diff` and read the changed files. +2. Report each finding with its file and line. + diff --git a/tests/fixtures/adapter-contracts/cursor-rule/contract-demo.mdc b/tests/fixtures/adapter-contracts/cursor-rule/contract-demo.mdc new file mode 100644 index 0000000..3ae2d78 --- /dev/null +++ b/tests/fixtures/adapter-contracts/cursor-rule/contract-demo.mdc @@ -0,0 +1,13 @@ +--- +description: "Contract fixture: a synthetic review skill that pins every adapter's rendered bytes, long enough that YAML folds it onto a second line." +alwaysApply: false +--- + +# Contract demo + +A synthetic skill for the adapter contract tests — it is never installed for +real. Non-ASCII text (naïve café, 検査) must reach every agent as UTF-8. + +1. Run `git diff` and read the changed files. +2. Report each finding with its file and line. + diff --git a/tests/fixtures/adapter-contracts/cursor/SKILL.md b/tests/fixtures/adapter-contracts/cursor/SKILL.md new file mode 100644 index 0000000..5ae88d0 --- /dev/null +++ b/tests/fixtures/adapter-contracts/cursor/SKILL.md @@ -0,0 +1,14 @@ +--- +name: contract-demo +description: 'Contract fixture: a synthetic review skill that pins every adapter''s + rendered bytes, long enough that YAML folds it onto a second line.' +--- + +# Contract demo + +A synthetic skill for the adapter contract tests — it is never installed for +real. Non-ASCII text (naïve café, 検査) must reach every agent as UTF-8. + +1. Run `git diff` and read the changed files. +2. Report each finding with its file and line. + diff --git a/tests/fixtures/adapter-contracts/kiro-steering/contract-demo.md b/tests/fixtures/adapter-contracts/kiro-steering/contract-demo.md new file mode 100644 index 0000000..8e33a1c --- /dev/null +++ b/tests/fixtures/adapter-contracts/kiro-steering/contract-demo.md @@ -0,0 +1,12 @@ +--- +inclusion: manual +--- + +# Contract demo + +A synthetic skill for the adapter contract tests — it is never installed for +real. Non-ASCII text (naïve café, 検査) must reach every agent as UTF-8. + +1. Run `git diff` and read the changed files. +2. Report each finding with its file and line. + diff --git a/tests/fixtures/adapter-contracts/kiro/SKILL.md b/tests/fixtures/adapter-contracts/kiro/SKILL.md new file mode 100644 index 0000000..5ae88d0 --- /dev/null +++ b/tests/fixtures/adapter-contracts/kiro/SKILL.md @@ -0,0 +1,14 @@ +--- +name: contract-demo +description: 'Contract fixture: a synthetic review skill that pins every adapter''s + rendered bytes, long enough that YAML folds it onto a second line.' +--- + +# Contract demo + +A synthetic skill for the adapter contract tests — it is never installed for +real. Non-ASCII text (naïve café, 検査) must reach every agent as UTF-8. + +1. Run `git diff` and read the changed files. +2. Report each finding with its file and line. + diff --git a/tests/fixtures/adapter-contracts/skill/contract-demo/meta.yaml b/tests/fixtures/adapter-contracts/skill/contract-demo/meta.yaml new file mode 100644 index 0000000..45f71ed --- /dev/null +++ b/tests/fixtures/adapter-contracts/skill/contract-demo/meta.yaml @@ -0,0 +1,10 @@ +name: contract-demo +description: "Contract fixture: a synthetic review skill that pins every adapter's rendered bytes, long enough that YAML folds it onto a second line." +category: testing +version: "1.2.3" +supported-agents: + - claude + - codex + - copilot + - cursor + - kiro diff --git a/tests/fixtures/adapter-contracts/skill/contract-demo/skill.md b/tests/fixtures/adapter-contracts/skill/contract-demo/skill.md new file mode 100644 index 0000000..bfc8667 --- /dev/null +++ b/tests/fixtures/adapter-contracts/skill/contract-demo/skill.md @@ -0,0 +1,7 @@ +# Contract demo + +A synthetic skill for the adapter contract tests — it is never installed for +real. Non-ASCII text (naïve café, 検査) must reach every agent as UTF-8. + +1. Run `git diff` and read the changed files. +2. Report each finding with its file and line. diff --git a/tests/test_adapter_contracts.py b/tests/test_adapter_contracts.py new file mode 100644 index 0000000..07d2253 --- /dev/null +++ b/tests/test_adapter_contracts.py @@ -0,0 +1,249 @@ +"""Adapter contract tests (#79). + +``tests/fixtures/adapter-contracts/`` pins, for every adapter, the exact bytes +that installing one synthetic skill writes (stamp included), where they go at +each scope, and what the environment variables that move an agent's config +directory do to that. ``docs/compatibility.md`` publishes the compatibility +matrix these contracts back and embeds a digest of the fixtures, so a format or +location change can't land without updating the fixtures, the matrix and the +changelog together. + +After an intended format change, regenerate the expected files with +``SKILLDECK_UPDATE_CONTRACTS=1 uv run --locked --extra dev pytest +tests/test_adapter_contracts.py``, review the fixture diff, then copy the new +digest the digest test reports into the matrix and the changelog. +""" + +import hashlib +import json +import os +from pathlib import Path + +import pytest +import yaml + +from skilldeck.adapters import ADAPTERS, ALL_ADAPTERS, InstallState +from skilldeck.registry import SkillError, load_skill +from skilldeck.stamp import parse as parse_stamp +from skilldeck.targets import Scope + +_ROOT = Path(__file__).resolve().parent.parent +FIXTURES = _ROOT / "tests" / "fixtures" / "adapter-contracts" +MATRIX = _ROOT / "docs" / "compatibility.md" +CHANGELOG = _ROOT / "CHANGELOG.md" + +#: set to 1 to rewrite the expected files from the current adapters +_UPDATE = os.environ.get("SKILLDECK_UPDATE_CONTRACTS") == "1" + +CONTRACTS = json.loads((FIXTURES / "contracts.json").read_text(encoding="utf-8")) +SKILL = load_skill(FIXTURES / "skill" / CONTRACTS["skill"], known_agents=ADAPTERS) + +#: (adapter, variable, case) for every environment case in the contracts +ENV_CASES = [ + (name, var, case) + for name, contract in sorted(CONTRACTS["adapters"].items()) + for var, cases in sorted(contract["env"].items()) + for case in sorted(cases) +] + + +def _fixture_files() -> list[Path]: + return sorted( + (path for path in FIXTURES.rglob("*") if path.is_file()), + key=lambda path: path.relative_to(FIXTURES).as_posix(), + ) + + +def contract_digest() -> str: + """sha256 over every fixture file's POSIX relative path and exact bytes.""" + digest = hashlib.sha256() + for path in _fixture_files(): + data = path.read_bytes() + rel = path.relative_to(FIXTURES).as_posix() + digest.update(f"{rel}\0{len(data)}\0".encode()) + digest.update(data) + return digest.hexdigest() + + +def _expected_file(name: str) -> Path: + """The fixture holding ``name``'s rendered file, byte for byte.""" + return FIXTURES / name / Path(CONTRACTS["adapters"][name]["project"]).name + + +def _check_bytes(name: str, dest: Path) -> None: + expected = _expected_file(name) + actual = dest.read_bytes() + if _UPDATE: + expected.parent.mkdir(parents=True, exist_ok=True) + expected.write_bytes(actual) + assert actual == expected.read_bytes(), ( + f"{name} renders {CONTRACTS['skill']} differently from its contract " + f"({expected.relative_to(_ROOT).as_posix()}). If the format change is " + "intended, regenerate the fixtures (see this module's docstring) and " + "update docs/compatibility.md and CHANGELOG.md" + ) + + +def _frontmatter(text: str) -> object: + assert text.startswith("---\n") + return yaml.safe_load(text.split("---\n")[1]) + + +@pytest.fixture +def home(tmp_path, monkeypatch): + home = tmp_path / "home" + monkeypatch.setattr(Path, "home", classmethod(lambda cls: home)) + return home + + +def test_every_adapter_has_a_contract(): + assert set(CONTRACTS["adapters"]) == set(ALL_ADAPTERS), ( + "add (or remove) the adapter's entry in " + "tests/fixtures/adapter-contracts/contracts.json and its row in " + "docs/compatibility.md" + ) + dirs = {path.name for path in FIXTURES.iterdir() if path.is_dir()} + assert dirs - {"skill"} == set(ALL_ADAPTERS) + + +def test_fixtures_have_lf_line_endings(): + # The byte comparisons need the fixtures exactly as committed; a checkout + # that converts them to CRLF (core.autocrlf on Windows) breaks them. + # .gitattributes marks the directory -text to prevent that. + crlf = [ + path.relative_to(FIXTURES).as_posix() + for path in _fixture_files() + if b"\r" in path.read_bytes() + ] + assert not crlf, f"fixtures were checked out with CRLF line endings: {crlf}" + + +@pytest.mark.parametrize("name", sorted(ALL_ADAPTERS)) +def test_project_install_matches_the_contract(name, tmp_path): + contract = CONTRACTS["adapters"][name] + adapter = ALL_ADAPTERS[name] + project = tmp_path.resolve() + dest = adapter.install(SKILL, Scope.PROJECT, project_root=project) + assert dest.relative_to(project).as_posix() == contract["project"] + assert adapter.relative_path(SKILL).as_posix() == contract["project"] + _check_bytes(name, dest) + + text = dest.read_text(encoding="utf-8") + found = parse_stamp(text) + assert found is not None + assert (found.name, found.version, found.modified) == ( + SKILL.name, + SKILL.version, + False, + ) + assert adapter.inspect(SKILL, Scope.PROJECT, project)[0] is InstallState.CURRENT + assert _frontmatter(text) == contract["frontmatter"] + if adapter.creates_skill_dir: + # Agent Skills: the frontmatter name must match the skill's folder + assert contract["frontmatter"]["name"] == dest.parent.name + + +@pytest.mark.parametrize("name", sorted(ALL_ADAPTERS)) +def test_global_install_matches_the_contract(name, tmp_path, home): + contract = CONTRACTS["adapters"][name] + adapter = ALL_ADAPTERS[name] + if contract["global"] is None: + assert adapter.scopes == (Scope.PROJECT,) + with pytest.raises(SkillError) as excinfo: + adapter.install(SKILL, Scope.GLOBAL) + message = str(excinfo.value) + # actionable: names what to use instead + for text in contract["global_error"]: + assert text in message + assert not home.exists() + return + assert contract["global"].startswith("~/") + dest = adapter.install(SKILL, Scope.GLOBAL) + assert "~/" + dest.relative_to(home).as_posix() == contract["global"] + _check_bytes(name, dest) + + +def test_env_contract_covers_every_variable_an_adapter_reads(): + for name, adapter in ALL_ADAPTERS.items(): + var = adapter.global_dir.env if adapter.global_dir else None + declared = set(CONTRACTS["adapters"][name]["env"]) + if var is not None: + assert var in declared, f"{name} reads {var}; add it to its contract" + assert {"absolute", "empty", "relative"} <= set( + CONTRACTS["adapters"][name]["env"][var] + ) + + +@pytest.mark.parametrize("name,var,case", ENV_CASES) +def test_env_overrides_match_the_contract(name, var, case, tmp_path, home, monkeypatch): + adapter = ALL_ADAPTERS[name] + moved = tmp_path / "moved-config" + value = {"absolute": str(moved), "empty": "", "relative": "relative/dir"}[case] + monkeypatch.setenv(var, value) + expected = CONTRACTS["adapters"][name]["env"][var][case] + if expected == "error": + with pytest.raises(SkillError, match=var): + adapter.install(SKILL, Scope.GLOBAL) + return + dest = adapter.install(SKILL, Scope.GLOBAL) + if expected.startswith(f"${var}/"): + assert dest.relative_to(moved).as_posix() == expected[len(var) + 2 :] + else: + assert "~/" + dest.relative_to(home).as_posix() == expected + _check_bytes(name, dest) + + +def _matrix_row(name: str) -> str: + rows = [ + line + for line in MATRIX.read_text(encoding="utf-8").splitlines() + if line.startswith(f"| `{name}` |") + ] + assert len(rows) == 1, f"docs/compatibility.md needs one matrix row for {name}" + return rows[0] + + +@pytest.mark.parametrize("name", sorted(ALL_ADAPTERS)) +def test_matrix_lists_each_adapter_with_its_contract_paths(name): + contract = CONTRACTS["adapters"][name] + row = _matrix_row(name) + placeholder = CONTRACTS["skill"] + assert f"`{contract['project'].replace(placeholder, '')}`" in row + if contract["global"] is not None: + assert f"`{contract['global'].replace(placeholder, '')}`" in row + for var, cases in contract["env"].items(): + if cases["absolute"].startswith(f"${var}/"): + assert f"`{var}`" in row + + +def test_matrix_scope_error_example_is_current(): + # docs/compatibility.md quotes the error for an unsupported scope + messages = set() + for adapter in ALL_ADAPTERS.values(): + if Scope.GLOBAL not in adapter.scopes: + with pytest.raises(SkillError) as excinfo: + adapter.check_scope(Scope.GLOBAL) + messages.add(f"error: {excinfo.value}") + quoted = [ + line + for line in MATRIX.read_text(encoding="utf-8").splitlines() + if line.startswith("error: ") + ] + assert quoted + assert set(quoted) <= messages + + +def test_matrix_and_changelog_record_the_contract_digest(): + digest = contract_digest() + line = f"" + assert line in MATRIX.read_text(encoding="utf-8"), ( + "the adapter contract fixtures changed: an adapter's output format or " + "location is different. Update the affected rows of " + "docs/compatibility.md (status, paths, verification date), replace its " + f"adapter-contract line with\n{line}\nand record the change in " + f"CHANGELOG.md under [Unreleased], mentioning `sha256:{digest[:12]}`" + ) + assert f"sha256:{digest[:12]}" in CHANGELOG.read_text(encoding="utf-8"), ( + "record the adapter contract change in CHANGELOG.md under " + f"[Unreleased], mentioning `sha256:{digest[:12]}`" + ) diff --git a/tests/test_adapters.py b/tests/test_adapters.py index 3b1691e..aa6c7c5 100644 --- a/tests/test_adapters.py +++ b/tests/test_adapters.py @@ -734,6 +734,15 @@ def test_check_scope(skill): LEGACY_ADAPTERS["cursor-rule"].check_scope(Scope.GLOBAL) +def test_scope_error_without_an_alternative(monkeypatch): + # a project-only adapter whose agent has no global location either can + # only point at the scope it does have + monkeypatch.setattr(ADAPTERS["cursor"], "global_dir", None) + with pytest.raises(SkillError) as excinfo: + LEGACY_ADAPTERS["cursor-rule"].check_scope(Scope.GLOBAL) + assert str(excinfo.value).endswith("for that scope. Use --scope project") + + def test_write_atomic_writes_lf_on_every_platform(tmp_path): # installs must be byte-identical across operating systems; text mode on # Windows would otherwise translate each "\n" to "\r\n" diff --git a/tests/test_cli.py b/tests/test_cli.py index b6a2705..9009b7b 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -470,6 +470,8 @@ def test_explicit_agent_without_the_scope_is_an_error(tmp_path, monkeypatch): ) assert result.exit_code == 1 assert "error: cursor-rule does not support --scope global" in result.output + # actionable: names the scope that works and the agent's native adapter + assert "Use --scope project, or --agent cursor (Agent Skills)" in result.output assert "security-review" in result.output # codex still reported From d57e73ee9c60427e79e020769c8248ea71328d2e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 09:35:02 +0000 Subject: [PATCH 2/2] Address review of the compatibility matrix and adapter contracts - Only install's unsupported-scope error suggests the native adapter (--agent cursor). For status, uninstall and update, that adapter would act on different files, so those commands name only --scope project. Adapter.install checks the scope first, so direct API use gets the same message. - The contract fixture directory must hold exactly the contract's files. The digest covers only those files, and covers contracts.json as canonical JSON without its _about notes. - The matrix test now checks each row cell by cell: - project and global paths, with "not supported" and "n/a" for project-only adapters - "Moved by" names exactly the variables that move the global install, each with its target, and mentions the ones that don't - both quoted scope errors match the real messages The docs and CLAUDE.md say what is enforced and what is kept by hand. - The CHANGELOG digest mention must be in [Unreleased] or the newest dated section, so the check still holds right after prepare_release cuts a release. Tested with synthetic changelogs and with the real cut_changelog. - Cursor and cursor-rule are experimental: their only evidence is @cursor/sdk. Experimental is now defined to cover SDK-only evidence. Claims about the Cursor app and CLI, including env overrides, are marked unverified, and cursor-rule's invocation gets the same caveat. - Codex's $name resolution is hedged: its two selection paths differ, and which surface uses which is unverified. docs/adapters.md is hedged too. Part of #79. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01HtiCGzpikMrkDYBkfQG5CX --- CHANGELOG.md | 25 +- CLAUDE.md | 11 +- docs/adapters.md | 8 +- docs/compatibility.md | 135 ++++++---- src/skilldeck/adapters/base.py | 12 +- src/skilldeck/cli.py | 16 +- .../fixtures/adapter-contracts/contracts.json | 39 ++- tests/test_adapter_contracts.py | 237 +++++++++++++++--- tests/test_adapters.py | 26 +- tests/test_cli.py | 36 ++- 10 files changed, 428 insertions(+), 117 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 55db3cd..a9ad883 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -151,7 +151,8 @@ All notable changes to this project are documented here. The format is based on - Asking an adapter for a scope it doesn't support now tells you what works instead (#79). For example, `--agent cursor-rule --scope global` names - `--scope project`, or `--agent cursor`, which supports `--scope global`. + `--scope project`. On `install` it also names `--agent cursor`, which + supports `--scope global`. - Five review skills cover defect classes their checklists missed, each cited to a fetched source (#104): - `ci-workflow-review` 0.4.0: newline injection through writes to @@ -512,21 +513,27 @@ All notable changes to this project are documented here. The format is based on - 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: + - a status (tested, supported or experimental; the page defines each) - project and global paths, and the environment variables that move them - how you invoke the skill in that agent - the minimum agent version - the date, agent version and vendor sources it was checked against + Whatever couldn't be checked is marked unverified. The page also says how skilldeck responds when a vendor deprecates or moves a skill location or - format. Adapter contract fixtures (`tests/fixtures/adapter-contracts/`, - contract `sha256:b3cbf84dafa6`) pin each adapter's exact rendered file, - stamp included, for one synthetic skill. They also pin its project and - global paths, and its behaviour with each config-directory variable set to - an absolute path, left empty, or set to a relative path. CI installs with - every adapter and compares the results byte for byte. A test fails when - the fixtures change unless the matrix's `adapter-contract` digest and this - changelog are updated too. + format. + + Adapter contract fixtures (`tests/fixtures/adapter-contracts/`, contract + `sha256:25b3bbad35d7`) pin each adapter's exact rendered file, stamp included, for + one synthetic skill. They also pin its project and global paths, and its + behaviour with each config-directory variable set to an absolute path, + left empty, or set to a relative path. CI installs with every adapter and + compares the results byte for byte. It also checks the matrix's path, + "Moved by" and scope-error text against the contracts. A test fails when + the fixtures change unless the matrix's `adapter-contract` digest is + updated too, and this changelog mentions the new digest under + `[Unreleased]` (or, just after a release, in its dated section). - `frontend-security-review` skill (0.1.0) (#105) — reviews browser-side changes (React, Vue, Angular, Svelte, plain JavaScript and HTML templates, CSP and header config): framework escape hatches (`dangerouslySetInnerHTML`, diff --git a/CLAUDE.md b/CLAUDE.md index 8634d42..380dbc3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -102,10 +102,13 @@ by `--agent all`); `skilldeck migrate` moves old-format installs to `SKILL.md`. - A skill's `meta.yaml` `name` must match its directory name; all metadata fields are required and validated by the registry. - Any change to an adapter's output format, paths or env handling must update - its contract fixtures, the matrix rows and `adapter-contract` digest in - `docs/compatibility.md`, and CHANGELOG (the contract tests enforce all three; - see `docs/compatibility.md#contract-tests`). Only state vendor behaviour a - primary source confirms; mark the rest *unverified*. + its contract fixtures, the `adapter-contract` digest and matrix rows in + `docs/compatibility.md`, and CHANGELOG. The contract tests enforce the + fixtures, the digest line, a digest mention in CHANGELOG, and the matrix's + path/"Moved by"/scope-error text. Status, minimum versions, dates and notes + are maintained by hand (see `docs/compatibility.md#contract-tests`). Only + state vendor behaviour a primary source confirms; mark the rest + *unverified*. - New skills follow the structural template (enforced by `tests/test_skill_structure.py`), ground their checklists in **fetched** authoritative sources (OWASP/CIS/vendor docs) cited in the skill body, and diff --git a/docs/adapters.md b/docs/adapters.md index b32834b..2874031 100644 --- a/docs/adapters.md +++ b/docs/adapters.md @@ -116,10 +116,12 @@ So one skill can be found more than once: - **Cursor** always reads `.agents/skills`, and also `.claude/skills` when third-party extensibility is on. It keeps the first copy per name, in the order `.cursor`, `.claude`, `.codex`, `.grok`, `.agents`. -- **Codex** keeps every copy it finds, and a plain `$name` mention only - resolves when exactly one enabled skill has that name. Installing a skill +- **Codex** keeps every copy it finds. A plain `$name` mention may then not + resolve: one of Codex's two skill-selection paths accepts it only when + exactly one enabled skill has that name, and the other takes the first + match. Which Codex surfaces use which path is unverified. Installing a skill for Codex at both project and global scope (or next to a copy of your own - in `.codex/skills` or `~/.codex/skills`) breaks `$name` for it. + in `.codex/skills` or `~/.codex/skills`) can break `$name` for it. - **An old format next to a skill** is not merged at all: VS Code lists a Copilot prompt file and a skill of the same name as two `/name` commands, and Cursor loads both the rule and the skill. diff --git a/docs/compatibility.md b/docs/compatibility.md index 35652cf..1e55179 100644 --- a/docs/compatibility.md +++ b/docs/compatibility.md @@ -1,6 +1,6 @@ # Agent compatibility - + This page lists what skilldeck installs for each agent and where it goes. It also covers how you then use a skill in that agent, the agent version it needs, @@ -15,16 +15,22 @@ checked against a vendor source is marked *unverified*. - **tested**: the agent was run at the listed version and seen to load a skill installed at these locations. -- **supported**: the vendor's own source code, shipped binary or documentation - at the listed version confirms the locations, format and variables. The - agent itself was not run against them. -- **experimental**: skilldeck installs as described, but a vendor source - contradicts part of the row, or the format is deprecated or doesn't load on - some of the agent's surfaces. Read the notes before relying on it. - -Status covers the agent side. Every adapter's own side is tested the same way, -whatever its status: CI checks the exact bytes, paths and stamp it writes on -every pull request, on Linux, macOS and Windows (see +- **supported**: the agent's own source code, shipped binary or vendor + documentation at the listed version confirms the locations, format and + variables. The agent itself was not run against them. +- **experimental**: skilldeck installs as described, but one of these holds: + - a vendor source contradicts part of the row + - the format is deprecated, or doesn't load on some of the agent's surfaces + - the only evidence is a related vendor package, such as an SDK that + bundles the agent's loader, rather than the agent or its documentation + + Read the notes before relying on it. + +Status covers the agent side, and is kept up to date by hand, like the +minimum versions, the dates and the notes. Every adapter's own side is +tested the same way, whatever its status: on every pull request, on Linux, +macOS and Windows, CI checks the exact bytes, paths and stamp it writes, and +checks this page's path, "Moved by" and scope-error text against them (see [Contract tests](#contract-tests)). ## Matrix @@ -36,10 +42,10 @@ every pull request, on Linux, macOS and Windows (see | `claude` | tested | `.claude/skills//SKILL.md` | `~/.claude/skills//SKILL.md` | `CLAUDE_CONFIG_DIR`, to `$CLAUDE_CONFIG_DIR/skills`; an empty value is refused | Claude Code 2.0.20 (project skills fixed in 2.0.24) | 2026-09-23, Claude Code 2.1.281 | | `codex` | supported | `.agents/skills//SKILL.md` | `~/.agents/skills//SKILL.md` | nothing (`CODEX_HOME` doesn't move it) | Codex 0.95.0 | 2026-09-23, Codex source at `rust-v0.156.1` | | `copilot` | supported | `.github/skills//SKILL.md` | `~/.copilot/skills//SKILL.md` | `COPILOT_HOME`, to `$COPILOT_HOME/skills` (Copilot CLI only) | VS Code 1.109; Copilot CLI 0.0.371; the cloud agent and code review have no version | 2026-09-23, GitHub docs, VS Code source, Copilot CLI changelog up to 1.0.88 | -| `cursor` | supported | `.cursor/skills//SKILL.md` | `~/.cursor/skills//SKILL.md` | nothing | *unverified* | 2026-09-23, the skills loader in `@cursor/sdk` 1.0.32 (the desktop app was not inspected) | +| `cursor` | experimental | `.cursor/skills//SKILL.md` | `~/.cursor/skills//SKILL.md` | nothing in `@cursor/sdk`; the Cursor app and CLI are *unverified* | *unverified* | 2026-09-23, the skills loader in `@cursor/sdk` 1.0.32 only | | `kiro` | supported | `.kiro/skills//SKILL.md` | `~/.kiro/skills//SKILL.md` | `KIRO_HOME`, to `$KIRO_HOME/skills` (Kiro CLI; the IDE is *unverified*) | *unverified* | 2026-09-23, Kiro CLI 2.24.0 (binary and embedded docs); the IDE from Kiro's docs only | | `copilot-prompt` | experimental | `.github/prompts/.prompt.md` | not supported | n/a | *unverified* | 2026-09-23, VS Code source and docs | -| `cursor-rule` | supported | `.cursor/rules/.mdc` | not supported | n/a | *unverified* | 2026-09-23, the rules loader in `@cursor/sdk` 1.0.32 | +| `cursor-rule` | experimental | `.cursor/rules/.mdc` | not supported | n/a | *unverified* | 2026-09-23, the rules loader in `@cursor/sdk` 1.0.32 only | | `kiro-steering` | experimental | `.kiro/steering/.md` | `~/.kiro/steering/.md` | `KIRO_HOME`, to `$KIRO_HOME/steering` (Kiro CLI; the IDE is *unverified*) | *unverified* | 2026-09-23, Kiro CLI 2.24.0 embedded docs; Kiro's docs mirror | The first five are the native adapters that `--agent all` selects. The last @@ -47,17 +53,25 @@ three are opt-in [legacy adapters](adapters.md#legacy-adapters) for agent versions that predate skills. When a variable in the "Moved by" column is set, skilldeck needs an absolute -path in it. It refuses a relative one, because the agent would resolve it against whatever -directory it was started in. `COPILOT_HOME` and `KIRO_HOME` count as unset +path in it. It refuses a relative one, because the agent would resolve it +against whatever directory it was started in. `COPILOT_HOME` and `KIRO_HOME` count as unset when empty. See [Environment variables](adapters.md#environment-variables). Asking `copilot-prompt` or `cursor-rule` for `--scope global` fails, and the -error names what works instead: +error names what works instead. `install` also points at the agent's native +adapter: ```text error: cursor-rule does not support --scope global: it has no stable file location for that scope. Use --scope project, or --agent cursor (Agent Skills), which supports --scope global ``` +`status`, `uninstall` and `update` name only `--scope project`, because the +native adapter's files are different ones: + +```text +error: cursor-rule does not support --scope global: it has no stable file location for that scope. Use --scope project +``` + ## Using a skill in each agent ### `claude`: Claude Code @@ -80,12 +94,16 @@ error: cursor-rule does not support --scope global: it has no stable file locati - **Invoked** by mentioning `$` in a prompt or picking the skill from `/skills` in the TUI. Codex also uses a skill when the task clearly - matches its description. A plain `$name` resolves only when exactly one - enabled skill has that name, so install each skill at one scope only. + matches its description. Codex keeps every copy of a skill it finds, so + install each skill at one scope only. With two copies, a plain `$name` + may not resolve: one of Codex's two skill-selection paths requires the + name to be unique, and the other takes the first match. Which Codex + surfaces use which path is *unverified*. - **Evidence**: `openai/codex@17cd2834`, whose skill-loading files match tag `rust-v0.156.1`: `codex-rs/ext/skills/src/host_roots.rs` lines 95–108 and 142–154, `codex-rs/skills/src/mentions.rs` line 41, - `codex-rs/skills/src/selection.rs` lines 188–190, + `codex-rs/skills/src/selection.rs` lines 188–190 (unique name required), + `codex-rs/ext/skills/src/selection.rs` lines 65–75 (first match), `codex-rs/ext/skills/src/catalog_prompt.rs` lines 3–8, and commits `39a6a84097` (`rust-v0.94.0`) and `e24058b7a8` (`rust-v0.95.0`). - **Unverified**: Codex's own skills documentation couldn't be fetched, and @@ -112,17 +130,25 @@ error: cursor-rule does not support --scope global: it has no stable file locati ### `cursor`: Cursor -- **Invoked**: Cursor offers each skill to the agent with its description, - and the agent decides when to use it. That choice is made on Cursor's - servers, so it is *unverified*. Cursor documents `/` only for skills - installed from a plugin; for skills in `.cursor/skills` it is *unverified*. +- **Invoked**: in the SDK's loader, each skill is offered to the agent with + its description, and the agent decides when to use it. That choice is made + on Cursor's servers, so it is *unverified*. Cursor documents `/` only + for skills installed from a plugin; for skills in `.cursor/skills` it is + *unverified*. +- **Experimental** because all the evidence comes from Cursor's + `@cursor/sdk` package, which bundles Cursor's own skills loader, and from + Cursor's SDK cookbook. Nobody checked that the Cursor app and the + `cursor-agent` CLI load skills from the same folders, or that no + environment variable moves them there. The SDK itself loads project and + user skills only when its `settingSources` option asks for them (none by + default). - **Evidence**: npm `@cursor/sdk` 1.0.32 `dist/esm/34.js` (the bundled skills loader's folder table, byte offset 552915); `cursor/cookbook@6733ef81` - `sdk/dag-task-runner/README.md` lines 146–152. + `sdk/dag-task-runner/README.md` lines 146–152. The only Cursor variable + found, `CURSOR_DATA_DIR`, moves `~/.cursor/projects`, not skills. - **Unverified**: Cursor's docs and changelog couldn't be reached, so the - first release with skills is unknown. Nobody checked that the desktop app - and the `cursor-agent` CLI use the SDK's loader unchanged. Cursor was not - run. + first release with skills is unknown, and so is the app's own behaviour. + Cursor was not run. ### `kiro`: Kiro @@ -161,15 +187,19 @@ error: cursor-rule does not support --scope global: it has no stable file locati ### `cursor-rule`: Cursor rules (legacy) -- **Invoked** when the agent asks for it: a rule with a `description` and - `alwaysApply: false` is pulled in when the description matches the task. +- **Invoked** when the agent asks for it: the SDK's loader treats a rule with + a `description` and `alwaysApply: false` as one the agent may request, and + offers it with its description. When the agent pulls it in is decided on + Cursor's servers, so it is *unverified*. +- **Experimental** for the same reason as `cursor`: the evidence comes from + `@cursor/sdk` only, not the Cursor app. - **Evidence**: npm `@cursor/sdk` 1.0.32 `dist/esm/34.js`, which loads `.cursor/rules/**/*.mdc` and reads `.mdc` frontmatter one line at a time (byte offset 565213). Running that reader showed that a folded description is cut off, which is why this adapter writes it on one line. -- **Unverified**: the minimum Cursor version, and whether the desktop app's - `.mdc` reader matches the SDK's. No vendor source deprecates `.mdc` rules, - but none rules it out either. +- **Unverified**: the minimum Cursor version, whether the Cursor app loads + rules the same way, and whether its `.mdc` reader matches the SDK's. No + vendor source deprecates `.mdc` rules, but none rules it out either. ### `kiro-steering`: Kiro steering files (legacy) @@ -197,21 +227,35 @@ The full list of sources, with paths and line numbers, is in - `skill/contract-demo/` is a small synthetic skill. Its description needs YAML quoting and folding, and its body has non-ASCII text. - `contracts.json` gives each adapter's project and global paths, the text - its `--scope global` error must contain if it is project-only, and its - expected frontmatter. It also says where a global install goes when each - environment variable is set to an absolute path, left empty, or set to a - relative path. + its `--scope global` error must contain if it is project-only (for + `install` and for other commands), and its expected frontmatter. It also + says where a global install goes when each environment variable is set to + an absolute path, left empty, or set to a relative path. - `/` holds the exact file the adapter installs, stamp included. -`tests/test_adapter_contracts.py` installs the skill with every adapter into -temporary directories and checks the result against these fixtures byte for -byte. The fixtures are committed with LF line endings (`.gitattributes`), -so the comparison is exact on Windows too. +The directory must hold exactly these files, and nothing else. + +`tests/test_adapter_contracts.py` enforces the following: + +- It installs the skill with every adapter into temporary directories and + checks the results against these fixtures byte for byte. The fixtures are + committed with LF line endings (`.gitattributes`), so the comparison is + exact on Windows too. +- It checks each matrix row's project and global cells against the contract, + and "not supported" and "n/a" for a project-only adapter. +- The "Moved by" cell must name exactly the variables that move the global + install, each with its target, and must mention any variable the contract + says doesn't move it. +- The quoted scope errors must match the real messages. + +It doesn't check status, minimum versions, the dates or the per-agent notes. +Those are kept up to date by hand. The `adapter-contract` comment at the top of this page is a SHA-256 digest of -the fixture directory. When an adapter's format or location changes, the -fixtures must change, and then the test fails until this page and -`CHANGELOG.md` are updated: +the contract files: the expected files, the synthetic skill, and +`contracts.json` without its `_about` notes. When an adapter's format or +location changes, the fixtures must change, and then the test fails until +this page and `CHANGELOG.md` are updated: 1. Regenerate the expected files with `SKILLDECK_UPDATE_CONTRACTS=1 uv run --locked --extra dev pytest tests/test_adapter_contracts.py`, @@ -220,7 +264,10 @@ fixtures must change, and then the test fails until this page and and agent version they were checked against. 3. Replace the `adapter-contract` line with the one the test prints. 4. Add a CHANGELOG entry under `[Unreleased]` that mentions the first 12 - characters of the digest (`sha256:<12 hex>`). The test checks for it. + characters of the digest (`sha256:<12 hex>`). The test looks for it in + `[Unreleased]` or in the newest dated section. The newest dated section + counts because cutting a release moves the entry there and leaves + `[Unreleased]` empty. An older section doesn't count. ## When a vendor changes a location or format diff --git a/src/skilldeck/adapters/base.py b/src/skilldeck/adapters/base.py index 4a990ae..57642f3 100644 --- a/src/skilldeck/adapters/base.py +++ b/src/skilldeck/adapters/base.py @@ -157,16 +157,19 @@ def scope_alternative(self, scope: Scope) -> str | None: worded for :meth:`check_scope`'s error; None if there is none.""" return None - def check_scope(self, scope: Scope) -> None: + def check_scope(self, scope: Scope, *, installing: bool = False) -> None: """Raise :class:`SkillError` if this agent cannot install at ``scope``. - The message names what does work: this adapter's other scope, and - any other adapter for the same agent that has ``scope``. + The message names what does work: this adapter's other scope and, + when ``installing``, another adapter for the same agent that has + ``scope``. That suggestion is for installs only: for ``status``, + ``uninstall`` or ``update`` another adapter would act on different + files from the ones asked about. """ if scope in self.scopes: return options = [f"--scope {other.value}" for other in self.scopes] - alternative = self.scope_alternative(scope) + alternative = self.scope_alternative(scope) if installing else None if alternative: options.append(alternative) hint = f". Use {', or '.join(options)}" if options else "" @@ -239,6 +242,7 @@ def install( *, force: bool = False, ) -> Path: + self.check_scope(scope, installing=True) dest = self.destination(skill, scope, project_root) mode = _entry_mode(dest) if mode is not None: diff --git a/src/skilldeck/cli.py b/src/skilldeck/cli.py index d9ecb58..403915e 100644 --- a/src/skilldeck/cli.py +++ b/src/skilldeck/cli.py @@ -62,7 +62,11 @@ def _resolve_skills(names: tuple[str, ...], select_all: bool) -> list[Skill]: def _resolve_adapters( - agents: tuple[str, ...], scope: Scope, everything: Collection[str] = ADAPTERS + agents: tuple[str, ...], + scope: Scope, + everything: Collection[str] = ADAPTERS, + *, + installing: bool = False, ) -> tuple[list[Adapter], bool]: """Turn ``--agent`` values into the adapters to run, deduped in order. @@ -70,8 +74,10 @@ def _resolve_adapters( adapters) that can install at ``scope``; the rest are skipped with a note. An agent named explicitly that can't is reported as an error instead, even alongside ``all``. So is an agent whose location at ``scope`` can't be - resolved, such as a relative ``CLAUDE_CONFIG_DIR``. Returns the adapters - and whether an error was reported. + resolved, such as a relative ``CLAUDE_CONFIG_DIR``. ``installing`` lets + the error for an unsupported scope suggest another adapter (see + :meth:`Adapter.check_scope`). Returns the adapters and whether an error + was reported. """ # dict.fromkeys dedupes, keeping order; a legacy adapter named alongside # ``all`` is not in ``everything`` but still runs @@ -84,7 +90,7 @@ def _resolve_adapters( for name in names: adapter = ALL_ADAPTERS[name] try: - adapter.check_scope(scope) + adapter.check_scope(scope, installing=installing) except SkillError as exc: if name in agents: # named explicitly click.echo(f"error: {exc}", err=True) @@ -269,7 +275,7 @@ def install( """Install one or more skills for the chosen agent(s).""" scope_enum = Scope(scope) skills = _resolve_skills(names, install_all) - adapters, failed = _resolve_adapters(agents, scope_enum) + adapters, failed = _resolve_adapters(agents, scope_enum, installing=True) for adapter in adapters: for skill in skills: if not adapter.supports(skill): diff --git a/tests/fixtures/adapter-contracts/contracts.json b/tests/fixtures/adapter-contracts/contracts.json index e86a015..48c50d4 100644 --- a/tests/fixtures/adapter-contracts/contracts.json +++ b/tests/fixtures/adapter-contracts/contracts.json @@ -1,13 +1,14 @@ { "_about": [ "The adapter contract: what installing skill/contract-demo writes for each adapter, and where.", - "project: path under the project root. global: path under ~, or null if the adapter is project-only,", - "in which case global_error lists text the --scope global error must contain.", + "project: path under the project root. global: path under ~, or null if the adapter is project-only.", + "global_error (project-only adapters): text the --scope global error must contain when installing,", + "and for other commands (status, uninstall, update), which must not suggest another --agent.", "env: per variable, where a global install goes when it is set to an absolute path ($VAR is its value),", "to an empty string, or to a relative path; \"error\" means skilldeck refuses.", "frontmatter: the rendered file's frontmatter, parsed as YAML.", "The expected file is /, byte for byte, stamp included.", - "Changing anything here changes the digest in docs/compatibility.md; see that page." + "The digest in docs/compatibility.md covers everything here except _about; see that page." ], "skill": "contract-demo", "adapters": { @@ -83,11 +84,17 @@ "copilot-prompt": { "project": ".github/prompts/contract-demo.prompt.md", "global": null, - "global_error": [ - "copilot-prompt does not support --scope global", - "--scope project", - "--agent copilot" - ], + "global_error": { + "install": [ + "copilot-prompt does not support --scope global", + "Use --scope project", + "--agent copilot (Agent Skills), which supports --scope global" + ], + "other": [ + "copilot-prompt does not support --scope global", + "Use --scope project" + ] + }, "env": {}, "frontmatter": { "description": "Contract fixture: a synthetic review skill that pins every adapter's rendered bytes, long enough that YAML folds it onto a second line.", @@ -97,11 +104,17 @@ "cursor-rule": { "project": ".cursor/rules/contract-demo.mdc", "global": null, - "global_error": [ - "cursor-rule does not support --scope global", - "--scope project", - "--agent cursor" - ], + "global_error": { + "install": [ + "cursor-rule does not support --scope global", + "Use --scope project", + "--agent cursor (Agent Skills), which supports --scope global" + ], + "other": [ + "cursor-rule does not support --scope global", + "Use --scope project" + ] + }, "env": {}, "frontmatter": { "description": "Contract fixture: a synthetic review skill that pins every adapter's rendered bytes, long enough that YAML folds it onto a second line.", diff --git a/tests/test_adapter_contracts.py b/tests/test_adapter_contracts.py index 07d2253..9040b22 100644 --- a/tests/test_adapter_contracts.py +++ b/tests/test_adapter_contracts.py @@ -12,11 +12,18 @@ ``SKILLDECK_UPDATE_CONTRACTS=1 uv run --locked --extra dev pytest tests/test_adapter_contracts.py``, review the fixture diff, then copy the new digest the digest test reports into the matrix and the changelog. + +These tests also check the matrix's path, "Moved by" and scope-error text +against the contracts. Status, minimum versions, the date checked and the +per-agent notes are maintained by hand. """ import hashlib +import importlib.util import json import os +import re +import shutil from pathlib import Path import pytest @@ -38,6 +45,9 @@ CONTRACTS = json.loads((FIXTURES / "contracts.json").read_text(encoding="utf-8")) SKILL = load_skill(FIXTURES / "skill" / CONTRACTS["skill"], known_agents=ADAPTERS) +#: the files a skill directory holds (``registry.load_skill``) +SKILL_FILES = ("meta.yaml", "skill.md") + #: (adapter, variable, case) for every environment case in the contracts ENV_CASES = [ (name, var, case) @@ -47,24 +57,64 @@ ] -def _fixture_files() -> list[Path]: - return sorted( - (path for path in FIXTURES.rglob("*") if path.is_file()), - key=lambda path: path.relative_to(FIXTURES).as_posix(), +def expected_fixture_files(root: Path = FIXTURES) -> list[str]: + """Every file the contracts in ``root`` call for, as sorted POSIX paths: + ``contracts.json``, the synthetic skill's files, and each adapter's + expected rendered file.""" + contracts = json.loads((root / "contracts.json").read_text(encoding="utf-8")) + files = {"contracts.json"} + files.update(f"skill/{contracts['skill']}/{name}" for name in SKILL_FILES) + files.update( + f"{name}/{Path(contract['project']).name}" + for name, contract in contracts["adapters"].items() + ) + return sorted(files) + + +def _canonical(rel: str, data: bytes) -> bytes: + """The bytes of fixture ``rel`` that the contract digest covers. + + ``contracts.json`` counts as its canonical JSON without the ``_about`` + notes, so rewording those, or reformatting the file, is not a contract + change. + """ + if rel != "contracts.json": + return data + contracts = json.loads(data.decode("utf-8")) + contracts.pop("_about", None) + canonical = json.dumps( + contracts, sort_keys=True, ensure_ascii=False, separators=(",", ":") ) + return canonical.encode("utf-8") -def contract_digest() -> str: - """sha256 over every fixture file's POSIX relative path and exact bytes.""" +def contract_digest(root: Path = FIXTURES) -> str: + """sha256 over the contract's fixture files: each one's POSIX relative + path and its bytes (see :func:`_canonical`).""" digest = hashlib.sha256() - for path in _fixture_files(): - data = path.read_bytes() - rel = path.relative_to(FIXTURES).as_posix() + for rel in expected_fixture_files(root): + data = _canonical(rel, (root / rel).read_bytes()) digest.update(f"{rel}\0{len(data)}\0".encode()) digest.update(data) return digest.hexdigest() +def changelog_records(text: str, mention: str) -> bool: + """Whether ``mention`` is in the changelog's ``[Unreleased]`` section or + its newest dated section. + + A contract change is recorded under ``[Unreleased]``. Cutting a release + (``scripts/prepare_release.py``) moves that entry into a new dated + section and leaves ``[Unreleased]`` empty, so the newest dated section + counts too; an older one doesn't. + """ + sections = re.split(r"^(?=## \[)", text, flags=re.MULTILINE) + unreleased = [s for s in sections if s.startswith("## [Unreleased]")] + dated = [s for s in sections if re.match(r"## \[\d", s)] + candidates = unreleased[:1] + dated[:1] + return any(mention in section for section in candidates) + + def _expected_file(name: str) -> Path: """The fixture holding ``name``'s rendered file, byte for byte.""" return FIXTURES / name / Path(CONTRACTS["adapters"][name]["project"]).name @@ -102,8 +152,22 @@ def test_every_adapter_has_a_contract(): "tests/fixtures/adapter-contracts/contracts.json and its row in " "docs/compatibility.md" ) - dirs = {path.name for path in FIXTURES.iterdir() if path.is_dir()} - assert dirs - {"skill"} == set(ALL_ADAPTERS) + + +def test_fixture_directory_holds_exactly_the_contract_files(): + # A stray file would otherwise sit there unchecked, and an expected file + # left behind by a renamed one would look like part of the contract. + actual = { + path.relative_to(FIXTURES).as_posix() + for path in FIXTURES.rglob("*") + if not path.is_dir() + } + expected = set(expected_fixture_files()) + assert actual == expected, ( + "tests/fixtures/adapter-contracts/ must hold exactly the contract's " + f"files; unexpected: {sorted(actual - expected)}, " + f"missing: {sorted(expected - actual)}" + ) def test_fixtures_have_lf_line_endings(): @@ -111,9 +175,9 @@ def test_fixtures_have_lf_line_endings(): # that converts them to CRLF (core.autocrlf on Windows) breaks them. # .gitattributes marks the directory -text to prevent that. crlf = [ - path.relative_to(FIXTURES).as_posix() - for path in _fixture_files() - if b"\r" in path.read_bytes() + rel + for rel in expected_fixture_files() + if b"\r" in (FIXTURES / rel).read_bytes() ] assert not crlf, f"fixtures were checked out with CRLF line endings: {crlf}" @@ -149,12 +213,17 @@ def test_global_install_matches_the_contract(name, tmp_path, home): adapter = ALL_ADAPTERS[name] if contract["global"] is None: assert adapter.scopes == (Scope.PROJECT,) + # actionable: names what to use instead. Only an install is pointed + # at another adapter, which would write different files. with pytest.raises(SkillError) as excinfo: adapter.install(SKILL, Scope.GLOBAL) - message = str(excinfo.value) - # actionable: names what to use instead - for text in contract["global_error"]: - assert text in message + for text in contract["global_error"]["install"]: + assert text in str(excinfo.value) + with pytest.raises(SkillError) as excinfo: + adapter.check_scope(Scope.GLOBAL) + for text in contract["global_error"]["other"]: + assert text in str(excinfo.value) + assert "--agent" not in str(excinfo.value) assert not home.exists() return assert contract["global"].startswith("~/") @@ -193,36 +262,72 @@ def test_env_overrides_match_the_contract(name, var, case, tmp_path, home, monke _check_bytes(name, dest) -def _matrix_row(name: str) -> str: +#: the matrix's columns, in order +MATRIX_COLUMNS = ( + "adapter", + "status", + "project", + "global", + "moved by", + "minimum version", + "last checked", +) + + +def _matrix_row(name: str) -> dict[str, str]: rows = [ line for line in MATRIX.read_text(encoding="utf-8").splitlines() if line.startswith(f"| `{name}` |") ] assert len(rows) == 1, f"docs/compatibility.md needs one matrix row for {name}" - return rows[0] + cells = [cell.strip() for cell in rows[0].strip().strip("|").split("|")] + assert len(cells) == len(MATRIX_COLUMNS), rows[0] + return dict(zip(MATRIX_COLUMNS, cells, strict=True)) @pytest.mark.parametrize("name", sorted(ALL_ADAPTERS)) -def test_matrix_lists_each_adapter_with_its_contract_paths(name): +def test_matrix_row_matches_the_contract(name): contract = CONTRACTS["adapters"][name] row = _matrix_row(name) - placeholder = CONTRACTS["skill"] - assert f"`{contract['project'].replace(placeholder, '')}`" in row - if contract["global"] is not None: - assert f"`{contract['global'].replace(placeholder, '')}`" in row + assert row["status"] in ("tested", "supported", "experimental") + + def shown(path: str) -> str: + return f"`{path.replace(CONTRACTS['skill'], '')}`" + + assert row["project"] == shown(contract["project"]) + if contract["global"] is None: + assert row["global"] == "not supported" + assert row["moved by"] == "n/a" + return + assert row["global"] == shown(contract["global"]) + + # "Moved by" names exactly the variables that move the global install, + # each with where it moves it, and mentions any that don't + entry = ALL_ADAPTERS[name].entry(SKILL).as_posix() + moving = {} for var, cases in contract["env"].items(): if cases["absolute"].startswith(f"${var}/"): - assert f"`{var}`" in row + moving[var] = cases["absolute"].removesuffix(f"/{entry}") + else: + assert f"`{var}`" in row["moved by"], f"say that {var} doesn't move it" + named = set(re.findall(r"`\$([A-Z][A-Z0-9_]*)[/`]", row["moved by"])) + assert named == set(moving) + for var, target in moving.items(): + assert row["moved by"].startswith(f"`{var}`, to `{target}`") + if not moving: + assert row["moved by"].startswith("nothing") def test_matrix_scope_error_example_is_current(): # docs/compatibility.md quotes the error for an unsupported scope messages = set() for adapter in ALL_ADAPTERS.values(): - if Scope.GLOBAL not in adapter.scopes: + if Scope.GLOBAL in adapter.scopes: + continue + for installing in (False, True): with pytest.raises(SkillError) as excinfo: - adapter.check_scope(Scope.GLOBAL) + adapter.check_scope(Scope.GLOBAL, installing=installing) messages.add(f"error: {excinfo.value}") quoted = [ line @@ -243,7 +348,79 @@ def test_matrix_and_changelog_record_the_contract_digest(): f"adapter-contract line with\n{line}\nand record the change in " f"CHANGELOG.md under [Unreleased], mentioning `sha256:{digest[:12]}`" ) - assert f"sha256:{digest[:12]}" in CHANGELOG.read_text(encoding="utf-8"), ( + mention = f"sha256:{digest[:12]}" + assert changelog_records(CHANGELOG.read_text(encoding="utf-8"), mention), ( "record the adapter contract change in CHANGELOG.md under " - f"[Unreleased], mentioning `sha256:{digest[:12]}`" + f"[Unreleased], mentioning `{mention}`" + ) + + +def test_contract_notes_are_not_part_of_the_digest(tmp_path): + copy = tmp_path / "contracts" + shutil.copytree(FIXTURES, copy) + path = copy / "contracts.json" + contracts = json.loads(path.read_text(encoding="utf-8")) + contracts["_about"] = ["reworded"] + path.write_text(json.dumps(contracts, indent=4), encoding="utf-8") + assert contract_digest(copy) == contract_digest() + contracts["adapters"]["claude"]["frontmatter"]["name"] = "changed" + path.write_text(json.dumps(contracts), encoding="utf-8") + assert contract_digest(copy) != contract_digest() + + +def test_digest_covers_only_the_contract_files(tmp_path): + copy = tmp_path / "contracts" + shutil.copytree(FIXTURES, copy) + (copy / "stray.txt").write_text("x", encoding="utf-8") + assert contract_digest(copy) == contract_digest() + with (copy / "kiro" / "SKILL.md").open("ab") as handle: + handle.write(b"x") + assert contract_digest(copy) != contract_digest() + + +_CHANGELOG = """# Changelog + +## [Unreleased] +{unreleased} +## [0.4.0] - 2026-10-01 + +### Added + +- {newest} + +## [0.3.0] - 2026-06-27 + +- {older} +""" + + +@pytest.mark.parametrize( + "where,recorded", + [ + ({"unreleased": "\n### Added\n\n- contract sha256:abcdef012345\n"}, True), + ({"newest": "contract sha256:abcdef012345"}, True), # just released + ({"older": "contract sha256:abcdef012345"}, False), + ({}, False), + ], +) +def test_changelog_records_the_digest_before_and_after_a_release(where, recorded): + text = _CHANGELOG.format( + **{"unreleased": "", "newest": "other", "older": "other", **where} + ) + assert changelog_records(text, "sha256:abcdef012345") is recorded + + +def test_cutting_a_release_keeps_the_digest_recorded(): + # the real changelog, through the real release-prep step + spec = importlib.util.spec_from_file_location( + "prepare_release", _ROOT / "scripts" / "prepare_release.py" ) + assert spec and spec.loader + prepare_release = importlib.util.module_from_spec(spec) + spec.loader.exec_module(prepare_release) + mention = f"sha256:{contract_digest()[:12]}" + text = CHANGELOG.read_text(encoding="utf-8") + released = prepare_release.cut_changelog(text, "99.0.0", "2099-01-01") + unreleased = released.split("## [Unreleased]")[1].split("## [")[0] + assert mention not in unreleased + assert changelog_records(released, mention) diff --git a/tests/test_adapters.py b/tests/test_adapters.py index aa6c7c5..5affc9c 100644 --- a/tests/test_adapters.py +++ b/tests/test_adapters.py @@ -734,13 +734,33 @@ def test_check_scope(skill): LEGACY_ADAPTERS["cursor-rule"].check_scope(Scope.GLOBAL) +def _scope_error(adapter, scope, **kwargs): + with pytest.raises(SkillError) as excinfo: + adapter.check_scope(scope, **kwargs) + return str(excinfo.value) + + +def test_scope_error_suggests_the_native_adapter_only_for_installs(): + # another adapter installs the skill elsewhere; for status, uninstall or + # update it would act on different files from the ones asked about + rule = LEGACY_ADAPTERS["cursor-rule"] + assert _scope_error(rule, Scope.GLOBAL).endswith( + "for that scope. Use --scope project" + ) + assert _scope_error(rule, Scope.GLOBAL, installing=True).endswith( + "Use --scope project, or --agent cursor (Agent Skills), which supports " + "--scope global" + ) + + def test_scope_error_without_an_alternative(monkeypatch): # a project-only adapter whose agent has no global location either can # only point at the scope it does have monkeypatch.setattr(ADAPTERS["cursor"], "global_dir", None) - with pytest.raises(SkillError) as excinfo: - LEGACY_ADAPTERS["cursor-rule"].check_scope(Scope.GLOBAL) - assert str(excinfo.value).endswith("for that scope. Use --scope project") + message = _scope_error( + LEGACY_ADAPTERS["cursor-rule"], Scope.GLOBAL, installing=True + ) + assert message.endswith("for that scope. Use --scope project") def test_write_atomic_writes_lf_on_every_platform(tmp_path): diff --git a/tests/test_cli.py b/tests/test_cli.py index 9009b7b..fedfcc7 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -470,11 +470,43 @@ def test_explicit_agent_without_the_scope_is_an_error(tmp_path, monkeypatch): ) assert result.exit_code == 1 assert "error: cursor-rule does not support --scope global" in result.output - # actionable: names the scope that works and the agent's native adapter - assert "Use --scope project, or --agent cursor (Agent Skills)" in result.output + # names the scope that works, but not another adapter: its status would + # be about different files + assert "for that scope. Use --scope project\n" in result.output + assert "--agent cursor" not in result.output assert "security-review" in result.output # codex still reported +@pytest.mark.parametrize("command", ["uninstall", "update"]) +def test_unsupported_scope_suggests_no_other_adapter_outside_install( + command, tmp_path, monkeypatch +): + monkeypatch.setattr(Path, "home", classmethod(lambda cls: tmp_path)) + args = [command, *(["logging"] if command == "uninstall" else [])] + result = CliRunner().invoke( + cli, [*args, "--agent", "cursor-rule", "--scope", "global"] + ) + assert result.exit_code == 1 + assert "for that scope. Use --scope project\n" in result.output + assert "--agent cursor" not in result.output + + +def test_unsupported_scope_on_install_suggests_the_native_adapter( + tmp_path, monkeypatch +): + monkeypatch.setattr(Path, "home", classmethod(lambda cls: tmp_path)) + result = CliRunner().invoke( + cli, ["install", "logging", "--agent", "cursor-rule", "--scope", "global"] + ) + assert result.exit_code == 1 + assert ( + "error: cursor-rule does not support --scope global: it has no stable " + "file location for that scope. Use --scope project, or --agent cursor " + "(Agent Skills), which supports --scope global\n" + ) in result.output + assert not (tmp_path / ".cursor").exists() + + def test_explicit_agent_without_the_scope_is_an_error_even_with_all( tmp_path, monkeypatch ):