Skip to content

Add skilldeck new and skilldeck validate for skill authors - #136

Merged
richardmhope merged 3 commits into
mainfrom
claude/codebase-review-o7y3i9
Sep 25, 2026
Merged

richardmhope merged 3 commits into
mainfrom
claude/codebase-review-o7y3i9

Conversation

@richardmhope

Copy link
Copy Markdown
Collaborator

What & why

Adds the skill-author commands from #72. A new skill starts out structurally complete, and the author gets precise local feedback before opening a PR.

  • skilldeck new NAME --category … writes:

    • a meta.yaml with every required field: version 0.1.0, every native agent unless --agent is given, and the read-only review capabilities baseline (git fetch/git diff/git ls-files plus the git remote), so the skill installs without a capabilities notice;
    • a skill.md skeleton in the shared review-skill structure: Scope diff steps, the finding format, the severity rubric word for word, verify-before-reporting, the findings cap and the report header. It writes a TODO(author) placeholder wherever domain content goes, and states no domain guidance or sources;
    • in a checkout, a skeleton under evals/fixtures/NAME/ as well (--no-eval-fixture skips it).

    It never overwrites anything, and refuses to write into an installed package.

  • skilldeck validate [NAME|PATH]... [--skills-dir] [--json] runs offline and checks:

    • metadata, including capabilities, via registry validation;
    • the bundle rules: a link, junction, executable or unexpected file in a skill directory. OS and editor leftovers are ignored;
    • structure, cited sources, placeholders and local links;
    • declared commands against the body's code spans, and that a git fetch declares the git remote;
    • rendering by every native and legacy adapter, and the catalog entry.

    In the checkout skilldeck itself runs from, it also checks eval fixtures (layout, keywords that echo the planted code, clean-fixture tolerance), the docs/finding-output.md rows, and whether generated output is stale. Each problem gives its file:line, a rule id and a fix. --json output is deterministic, and the command exits 0 only when everything is clean.

  • Trust. validate never runs code from the tree it checks. The fixture and generated-output checks import that checkout's own scripts, so they run only when the checkout is the running skilldeck's own source. For any other tree, including a fork, they're reported as skipped, with the uv run command to use inside it. Symlinked meta.yaml/skill.md files and linked skill directories are reported, never read. A regression test validates a look-alike tree whose scripts would write a marker file, and checks that nothing is written.

  • Placeholders. A fresh skeleton passes every metadata, structure, capability and rendering check, and is rated incomplete (not invalid) until its placeholders, a cited source, a planted eval fixture and a finding-output row exist. No passing citation is faked.

  • One set of rules. The structure, citation and command-declaration rules moved from the tests into skilldeck.lint, with a RULES table. The tests call it, a table-driven test pins every required piece, and the rule table in the docs is kept in sync with RULES. Registry errors now carry rule ids (with an optional line); their messages are unchanged.

  • Docs. docs/authoring-skills.md leads with the commands, lists every rule, and documents the review path for official and organization skills. It also notes that the full pytest suite still checks things validate can't, such as SAMPLE_REPORTS.

An independent review found that validate executed scripts from any tree that looked like a skilldeck checkout, which is exactly the organization-author case; that is fixed above. It also found symlink reads that could echo outside files, YAML errors without line numbers, and checks validate passed but pytest failed. All fixed.

Closes #72

Tracking: #94

Type of change

  • Bug fix
  • New feature
  • New or updated skill
  • Docs only
  • Refactor / internal

Checklist

  • Ran uv run --extra dev ruff check . && uv run --extra dev ruff format --check . && uv run --extra dev mypy && uv run --extra dev pytest (1189 passed, also with -W error::EncodingWarning and --resolution lowest-direct)
  • Added or updated tests
  • Updated docs where relevant
  • Added a CHANGELOG.md entry under ## [Unreleased]
  • For a skill change: bumped that skill's version in meta.yaml (no skill content changed)

🤖 Generated with Claude Code

https://claude.ai/code/session_01HtiCGzpikMrkDYBkfQG5CX


Generated by Claude Code

`skilldeck new NAME --category ...` scaffolds a skill: a meta.yaml with
every required field (version 0.1.0, every native agent unless --agent
narrows it) and a skill.md skeleton carrying the shared review-skill
structure (Scope diff steps, finding format, the severity rubric word for
word, verify-before-reporting, findings cap, report header). Wherever
domain content goes it writes a TODO(author) placeholder; it states no
domain guidance and cites no source. In a checkout it also scaffolds
evals/fixtures/NAME/ as a valid clean-diff fixture skeleton
(--no-eval-fixture skips it). It never overwrites anything.

`skilldeck validate [NAME|PATH]... [--skills-dir] [--json]` checks skills
offline: meta.yaml (registry validation), structure, cited sources,
leftover placeholders, stray files, rendering by every native and legacy
adapter, and the catalog entry. For a checkout's src/skilldeck/skills it
also checks eval fixtures (via the checkout's evals/run_evals.py loader),
the skill's rows in docs/finding-output.md, and generated-output freshness
(scripts/build_plugin.py --check logic, in process). Every problem names
its file and line, a rule id, and a remediation; --json is deterministic;
exit 0 only when clean, 2 on usage errors.

Placeholder policy: a fresh skeleton passes every metadata and structure
check, and is rated "incomplete" (not "invalid") while placeholders, a
cited source, a planted eval fixture or its finding-output row are
missing, rather than carrying a fake passing citation.

Outside a checkout both commands need an explicit directory (--dir,
--skills-dir) for organization skills, and new refuses to write into the
installed package.

The structure and citation rules move from the tests into skilldeck.lint
(with a RULES table of ids, levels and remediations, and a package copy of
the severity rubric that a test keeps identical to docs/finding-output.md);
test_skill_structure and test_skill_citations now call the package rules.
Registry SkillErrors carry the rule they break, with messages unchanged,
and replacement checks are available without raising. catalog.skill_entry
builds one entry without the packaged manifest.

docs/authoring-skills.md now leads with the commands, lists every rule (a
test keeps the table in sync), and documents the review path for official
and organization-specific skills. README, CONTRIBUTING, CLAUDE.md and
CHANGELOG updated.

Closes #72

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HtiCGzpikMrkDYBkfQG5CX
Address review findings on the skill author commands (#72):

- validate no longer runs code from the tree it checks. The eval-fixture
  and generated-output checks import a checkout's evals/run_evals.py and
  scripts/build_plugin.py, so they now run only when that checkout's
  src/skilldeck is the running package (_trusted_checkout); for any other
  tree that looks like a checkout (a fork, an archive, a checkout checked
  by an installed skilldeck) they are reported as skipped with the
  `uv run --extra dev skilldeck validate` command to run inside it. The
  text-only finding-output check still applies. A regression test builds a
  mimic tree whose scripts write a sentinel and validates it through cwd,
  --skills-dir, a path argument and a cwd inside the skills directory.
- Symlinks in a skill directory are reported (new rule skill.symlink,
  from lint.bundle_problems, which also owns skill.unexpected-file) and
  never read, so a link's target path and contents cannot reach the
  report; the other skill file is still checked (registry.check_meta
  validates meta.yaml alone). Paths are shown with os.path.abspath, so a
  symlinked skills directory keeps the name the user gave.
- A YAML syntax error in meta.yaml is reported on its line, with a
  one-line message and no PyYAML excerpt of the file; the registry's own
  message is unchanged.
- validate now also checks what the fixture tests did: plant keywords that
  echo the planted code (eval.keyword-echo) and a clean-diff fixture's
  tolerance (eval.clean-tolerance), through run_evals helpers that
  tests/test_eval_fixtures.py shares. `new`'s checkout steps and the
  authoring walkthrough now name the SAMPLE_REPORTS entry and running
  pytest before pushing.
- Nits: restore the 
 escape text in a registry comment; reject empty
  validate targets; "no errors in the skill" in the incomplete status
  line; shlex-quote the --skills-dir hint; test that removing each required
  structural piece fires its rule (and make the heading lookup independent
  of REQUIRED_HEADINGS); drop the now-unused report notes; rewrap CLAUDE.md
  and document the trust rule there and in docs/authoring-skills.md.

Part of #72

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HtiCGzpikMrkDYBkfQG5CX
Integrate the skill author commands with the capability manifest and the
bundle rules:

- `skilldeck new` writes the read-only review capabilities the bundled
  review skills declare (read: repo, write: none; git fetch, git diff,
  git ls-files; the git remote, worded as they word it), so the skeleton
  renders without a Declared capabilities notice and its Scope steps
  match its declaration both ways.
- The bundle rules stay in the registry: bundle_entries() reports each
  offending entry with its kind, bundle_problems() keeps its messages, and
  lint.bundle_problems maps kinds to rules (skill.link, which replaces
  skill.symlink and covers junctions and a linked skill directory;
  skill.executable; skill.unexpected-file; skill.unreadable). OS and
  editor leftovers are ignored, as when loading.
- The declared-commands-vs-code-spans check moves from
  tests/test_skill_structure.py into skilldeck.lint (KNOWN_PROGRAMS and
  MENTIONED_ONLY with it) as capabilities.undeclared-command,
  capabilities.unused-command and capabilities.network; the tests call it,
  as strictly as before.
- Registry errors carry rule ids for the new checks too (meta.capabilities,
  skill.link, references.local-link), and check_meta() returns the
  validated SkillMeta, so validate checks meta.yaml, the bundle and
  skill.md separately and still renders a skill whose only problems are
  bundle or local-link ones.
- Docs: the author walkthrough and `new` mention declaring capabilities;
  the rule table lists the new rules; CHANGELOG keeps main's structure
  with this entry under Added.

Part of #72

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HtiCGzpikMrkDYBkfQG5CX
@richardmhope
richardmhope merged commit 025ab81 into main Sep 25, 2026
17 checks passed
@richardmhope
richardmhope deleted the claude/codebase-review-o7y3i9 branch September 25, 2026 04:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add skill author scaffolding and one-command validation

2 participants