diff --git a/.buildkite/test_areas/misc.yaml b/.buildkite/test_areas/misc.yaml index dea2ca7a6f..602491e0e0 100644 --- a/.buildkite/test_areas/misc.yaml +++ b/.buildkite/test_areas/misc.yaml @@ -560,6 +560,8 @@ steps: - tools/pre_commit/check_test_tethering.py - tools/pre_commit/test_tethering_allowlist.txt - tests/tools/test_check_test_tethering.py + - tools/pre_commit/check_agent_files.py + - tests/tools/test_check_agent_files.py - tests/tool_parsers - tests/tokenizers_ - tests/parser @@ -609,6 +611,7 @@ steps: - pytest -v -s -m 'cpu_test' multimodal - pytest -v -s config - pytest -v -s tools/test_check_test_tethering.py + - pytest -v -s tools/test_check_agent_files.py - label: ":computer: (CPU) Reasoning + Renderers" key: cpu-reasoning-renderers diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 24dcf826fb..3e418e005e 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -30,7 +30,6 @@ repos: - id: markdownlint-cli2 language_version: lts args: [--config, tools/pre_commit/.markdownlint.yaml, --fix] - exclude: (^|/)CLAUDE\.md$ - repo: https://github.com/rhysd/actionlint rev: v1.7.7 hooks: @@ -177,6 +176,14 @@ repos: files: ^\.github/mergify\.yml$ pass_filenames: false additional_dependencies: [PyYAML, regex] + - id: check-agent-files + name: Check agent instruction files and skills + language: python + entry: python tools/pre_commit/check_agent_files.py + files: ^(\.agents/|\.claude/|(.*/)?(AGENTS|CLAUDE)\.md$|tools/pre_commit/check_agent_files\.py$) + pass_filenames: false + additional_dependencies: [PyYAML, regex] + verbose: true # `note:` lines are advisory, so the hook passes - id: check-buildkite-step-keys name: Check every Buildkite step declares an explicit key language: python diff --git a/docs/contributing/editing-agent-instructions.md b/docs/contributing/editing-agent-instructions.md index 7be5f594cd..2810b6fdca 100644 --- a/docs/contributing/editing-agent-instructions.md +++ b/docs/contributing/editing-agent-instructions.md @@ -44,6 +44,19 @@ cross-file context, and fixes for repeated mistakes. Each entry should be short, specific, and actionable — e.g., which files to touch, what order to change them in, and which tests to run. +## Skills + +Skills live in `.agents/skills//` and follow the +[Agent Skills specification](https://agentskills.io/specification). + +- Claude Code does not read `.agents/`, so symlink each skill: + `ln -s ../../.agents/skills/ .claude/skills/`. +- Frontmatter: `name` matches the directory. `description` says what the + skill does and when to use it; quote it if it contains a colon. +- Keep `SKILL.md` under 500 lines and move detail into files it links to. +- `tools/pre_commit/check_agent_files.py` runs in pre-commit to check the + skill and `AGENTS.md` rules + ## Keeping Docs Lean - Every addition should trigger review of surrounding content for stale or redundant items. diff --git a/rust/CLAUDE.md b/rust/CLAUDE.md deleted file mode 100644 index f96fd34544..0000000000 --- a/rust/CLAUDE.md +++ /dev/null @@ -1,4 +0,0 @@ -# CLAUDE.md - -First, check @AGENTS.override.md if exists. -Then, follow instructions in @AGENTS.md. diff --git a/rust/src/bench/CLAUDE.md b/rust/src/bench/CLAUDE.md deleted file mode 100644 index 43c994c2d3..0000000000 --- a/rust/src/bench/CLAUDE.md +++ /dev/null @@ -1 +0,0 @@ -@AGENTS.md diff --git a/tests/tools/test_check_agent_files.py b/tests/tools/test_check_agent_files.py new file mode 100644 index 0000000000..ec9d4c8e09 --- /dev/null +++ b/tests/tools/test_check_agent_files.py @@ -0,0 +1,129 @@ +# SPDX-License-Identifier: Apache-2.0 +# SPDX-FileCopyrightText: Copyright contributors to the vLLM project +"""Unit tests for the agent-file pre-commit check. + +The hook passes silently on a healthy tree, so CI would not notice a rule that +stopped firing. Each case breaks one convention in a minimal repo and asserts +the check reports it. +""" + +import os +from pathlib import Path + +import pytest + +from tools.pre_commit.check_agent_files import check + +SKILL_MD = ".agents/skills/demo/SKILL.md" +SKILL = "---\nname: demo\ndescription: Demo things. Use when testing.\n---\n" + + +def write(path: str, text: str) -> None: + Path(path).parent.mkdir(parents=True, exist_ok=True) + Path(path).write_text(text) + + +def tracked() -> dict[str, str]: + """Mimic `git ls-files -s` for every file and symlink under the cwd.""" + files = {} + for root, dirs, names in os.walk("."): + for name in dirs + names: + path = os.path.normpath(os.path.join(root, name)) + if os.path.islink(path): + files[path] = "120000" + elif os.path.isfile(path): + files[path] = "100644" + return files + + +@pytest.fixture(autouse=True) +def repo(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + write("AGENTS.md", "See the [guide](docs/guide.md#some-heading).\n") + write("docs/guide.md", "## Some Heading\n") + write(SKILL_MD, SKILL) + os.makedirs(".claude/skills") + os.symlink("../../.agents/skills/demo", ".claude/skills/demo") + + +def test_valid_repo_passes(): + assert check(tracked()) == ([], []) + + +@pytest.mark.parametrize( + ("break_repo", "expected"), + [ + pytest.param( + lambda: os.unlink(".claude/skills/demo"), + "must be a symlink", + id="skill-not-linked", + ), + pytest.param( + lambda: write(".claude/skills/other/SKILL.md", SKILL), + "only symlinks", + id="real-file-in-claude-skills", + ), + pytest.param( + lambda: os.symlink("../../.agents/skills/gone", ".claude/skills/gone"), + "only symlinks", + id="dangling-link", + ), + pytest.param( + lambda: write(SKILL_MD, SKILL.replace("demo", "other")), + "must be 'demo'", + id="name-differs-from-dir", + ), + pytest.param( + lambda: write(SKILL_MD, SKILL.replace("Demo things", "Demo: things")), + "not valid YAML", + id="unquoted-colon-in-description", + ), + pytest.param( + lambda: write(SKILL_MD, "# Demo\n"), + "missing YAML frontmatter", + id="no-frontmatter", + ), + pytest.param( + lambda: write("vllm/CLAUDE.md", "@AGENTS.md\n"), + "remove it", + id="claude-md", + ), + pytest.param( + lambda: write("AGENTS.md", "line\n" * 201), + "200-line budget", + id="root-guide-over-budget", + ), + pytest.param( + lambda: write("vllm/AGENTS.md", "line\n" * 301), + "300-line budget", + id="domain-guide-over-budget", + ), + pytest.param( + lambda: write(SKILL_MD, SKILL + "line\n" * 500), + "500-line budget", + id="skill-over-budget", + ), + pytest.param( + lambda: write("AGENTS.md", "[gone](docs/gone.md)\n"), + "does not exist", + id="dead-link", + ), + pytest.param( + lambda: write(SKILL_MD, SKILL + "[guide](../../../docs/guide.md#gone)\n"), + "no heading", + id="dead-anchor", + ), + ], +) +def test_broken_convention_is_reported(break_repo, expected): + break_repo() + errors, _ = check(tracked()) + assert len(errors) == 1 + assert expected in errors[0] + + +def test_description_without_trigger_is_advisory(): + write(SKILL_MD, SKILL.replace(" Use when testing.", "")) + errors, notes = check(tracked()) + assert not errors + assert "when to use" in notes[0] diff --git a/tools/pre_commit/check_agent_files.py b/tools/pre_commit/check_agent_files.py new file mode 100644 index 0000000000..90833f29d4 --- /dev/null +++ b/tools/pre_commit/check_agent_files.py @@ -0,0 +1,190 @@ +# SPDX-License-Identifier: Apache-2.0 +# SPDX-FileCopyrightText: Copyright contributors to the vLLM project +"""Check agent instruction files and skills against vLLM's conventions. + +Agents fail silently on a broken setup: a skill with no `.claude/skills` symlink +or with unparsable frontmatter is simply never offered, and a dead link reads as +missing guidance. See `docs/contributing/editing-agent-instructions.md` for the +rules enforced here. + +Usage: + python tools/pre_commit/check_agent_files.py +""" + +import os +import posixpath +import subprocess +import sys +from pathlib import Path + +import regex as re +import yaml + +SKILLS = ".agents/skills" +CLAUDE_SKILLS = ".claude/skills" +SYMLINK = "120000" +ROOT_GUIDE_MAX_LINES = 200 +DOMAIN_GUIDE_MAX_LINES = 300 +SKILL_MAX_LINES = 500 + +FRONTMATTER = re.compile(r"\A---\n(.*?)\n---\n", re.DOTALL) +SKILL_NAME = re.compile(r"[a-z0-9]+(?:-[a-z0-9]+)*") +WHEN_TO_USE = re.compile(r"(?i)\buse\b") +CODE_BLOCK = re.compile(r"```.*?```", re.DOTALL) +LINK = re.compile(r"(? dict[str, str]: + """Map each tracked path to its git mode.""" + out = subprocess.run( + ["git", "ls-files", "-s", "-z"], capture_output=True, text=True, check=True + ).stdout + files = {} + for entry in out.split("\0")[:-1]: + meta, path = entry.split("\t", 1) + files[path] = meta.split()[0] + return files + + +def link_target(path: str) -> str: + # Without core.symlinks, git checks a symlink out as a file holding its target. + return os.readlink(path) if os.path.islink(path) else Path(path).read_text() + + +def skill_names(files: dict[str, str]) -> list[str]: + return sorted({p.split("/")[2] for p in files if p.startswith(f"{SKILLS}/")}) + + +def check_skill_links(files: dict[str, str]) -> list[str]: + errors = [] + skills = skill_names(files) + for name in skills: + link = f"{CLAUDE_SKILLS}/{name}" + target = f"../../{SKILLS}/{name}" + if files.get(link) != SYMLINK or link_target(link) != target: + errors.append(f"{link}: must be a symlink to {target}") + for path, mode in files.items(): + if not path.startswith(f"{CLAUDE_SKILLS}/"): + continue + if mode != SYMLINK or path.split("/")[2] not in skills: + errors.append(f"{path}: only symlinks to {SKILLS}/ belong here") + return errors + + +def check_skill_frontmatter(files: dict[str, str]) -> tuple[list[str], list[str]]: + errors, notes = [], [] + for name in skill_names(files): + path = f"{SKILLS}/{name}/SKILL.md" + if path not in files: + errors.append(f"{SKILLS}/{name}: missing SKILL.md") + continue + match = FRONTMATTER.match(Path(path).read_text()) + try: + meta = yaml.safe_load(match.group(1)) if match else None + except yaml.YAMLError as exc: + reason = getattr(exc, "problem", None) or exc + errors.append(f"{path}: frontmatter is not valid YAML ({reason})") + continue + if not isinstance(meta, dict): + errors.append(f"{path}: missing YAML frontmatter") + continue + if meta.get("name") != name: + errors.append(f"{path}: name {meta.get('name')!r} must be {name!r}") + if not SKILL_NAME.fullmatch(name) or len(name) > 64: + errors.append(f"{path}: name must be lowercase-hyphenated, <= 64 chars") + description = meta.get("description") + if not isinstance(description, str) or not description.strip(): + errors.append(f"{path}: missing description") + elif len(description) > 1024: + errors.append(f"{path}: description exceeds 1024 chars") + elif not WHEN_TO_USE.search(description): + notes.append(f"{path}: description should say when to use the skill") + return errors, notes + + +def check_no_claude_md(files: dict[str, str]) -> list[str]: + return [ + f"{path}: remove it; Claude Code reads AGENTS.md, but not where a " + "CLAUDE.md exists" + for path in files + if posixpath.basename(path) == "CLAUDE.md" + ] + + +def check_line_budgets(files: dict[str, str]) -> list[str]: + errors = [] + for path in files: + if path == "AGENTS.md": + limit = ROOT_GUIDE_MAX_LINES + elif posixpath.basename(path) == "AGENTS.md": + limit = DOMAIN_GUIDE_MAX_LINES + elif path.startswith(f"{SKILLS}/") and path.endswith("/SKILL.md"): + limit = SKILL_MAX_LINES + else: + continue + lines = len(Path(path).read_text().splitlines()) + if lines > limit: + errors.append(f"{path}: {lines} lines exceeds the {limit}-line budget") + return errors + + +def anchor_key(text: str) -> str: + # Anchor slugs differ between renderers, so compare alphanumerics only. + return re.sub(r"[^a-z0-9]", "", text.lower()) + + +def has_heading(path: str, anchor: str) -> bool: + headings = HEADING.findall(CODE_BLOCK.sub("", Path(path).read_text())) + return anchor_key(anchor) in {anchor_key(heading) for heading in headings} + + +def check_links(files: dict[str, str]) -> list[str]: + errors = [] + dirs = {str(parent) for p in files for parent in Path(p).parents} + for path in files: + in_skill = path.startswith(f"{SKILLS}/") and path.endswith(".md") + if not in_skill and posixpath.basename(path) != "AGENTS.md": + continue + text = CODE_BLOCK.sub("", Path(path).read_text()) + for link in LINK.findall(text): + if URL_SCHEME.match(link): + continue + ref, _, anchor = link.partition("#") + dest = posixpath.normpath(posixpath.join(posixpath.dirname(path), ref)) + if dest not in files and dest not in dirs: + errors.append(f"{path}: link {link} does not exist") + elif anchor and dest.endswith(".md") and not has_heading(dest, anchor): + errors.append(f"{path}: link {link} has no heading #{anchor}") + return errors + + +def check(files: dict[str, str]) -> tuple[list[str], list[str]]: + """Return (errors, advisory notes) for the tracked files under the cwd.""" + errors, notes = check_skill_frontmatter(files) + errors += check_skill_links(files) + errors += check_no_claude_md(files) + errors += check_line_budgets(files) + errors += check_links(files) + return errors, notes + + +def main() -> int: + errors, notes = check(tracked_files()) + for note in notes: + print(f"note: {note}") + if not errors: + return 0 + print(f"{len(errors)} agent file problem(s):\n", file=sys.stderr) + for error in errors: + print(f" {error}", file=sys.stderr) + print( + "\nSee docs/contributing/editing-agent-instructions.md.", + file=sys.stderr, + ) + return 1 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/vllm/parser/CLAUDE.md b/vllm/parser/CLAUDE.md deleted file mode 100644 index 43c994c2d3..0000000000 --- a/vllm/parser/CLAUDE.md +++ /dev/null @@ -1 +0,0 @@ -@AGENTS.md diff --git a/vllm/reasoning/CLAUDE.md b/vllm/reasoning/CLAUDE.md deleted file mode 100644 index 43c994c2d3..0000000000 --- a/vllm/reasoning/CLAUDE.md +++ /dev/null @@ -1 +0,0 @@ -@AGENTS.md diff --git a/vllm/tool_parsers/CLAUDE.md b/vllm/tool_parsers/CLAUDE.md deleted file mode 100644 index 43c994c2d3..0000000000 --- a/vllm/tool_parsers/CLAUDE.md +++ /dev/null @@ -1 +0,0 @@ -@AGENTS.md