diff --git a/.agents/skills/align-recipe-pyproject/SKILL.md b/.agents/skills/align-recipe-pyproject/SKILL.md index 5a381e883e..44d9561375 100644 --- a/.agents/skills/align-recipe-pyproject/SKILL.md +++ b/.agents/skills/align-recipe-pyproject/SKILL.md @@ -52,7 +52,7 @@ Runs `scripts/align_pyproject.py` against a recipe directory. Six rules: 1. **Always use the script — never hand-edit `pyproject.toml` or `manifest.yaml` to perform these changes.** The script exists specifically so edits are style-preserving and reviewable via one report. -2. **Ask for the recipe directory** if the user has not provided one. Do not guess. Recipe roots live under `core/python//`, `contrib//`, or `skills/python//`. +2. **Ask for the recipe directory** if the user has not provided one. Do not guess. Recipe roots live under `core/python//`, `contrib/python//`, or `skills///`. 3. **Always start with `--dry-run`** unless the user has explicitly said "apply", "fix it", "just do it", or equivalent. Show them what would change before doing it. @@ -76,7 +76,7 @@ Runs `scripts/align_pyproject.py` against a recipe directory. Six rules: | Field | Required | Description | |---|---|---| -| `--recipe-dir` | Yes | Path to the recipe root (e.g. `core/python/cross-session-memory`, `contrib/my-recipe`, `skills/python/my-skill`). | +| `--recipe-dir` | Yes | Path to the recipe root (e.g. `core/python/cross-session-memory`, `contrib/python/my-recipe`, `skills/retail/store-ops`). | | `--dry-run` | No | Report what would change without modifying any files. | | `--description-source` | Only when resolving a `description-matches-manifest` mismatch. Values: `pyproject`, `manifest`, `delete`. See below. | Chooses how to reconcile a description mismatch. | diff --git a/.agents/skills/align-recipe-pyproject/scripts/align_pyproject.py b/.agents/skills/align-recipe-pyproject/scripts/align_pyproject.py index 9d9b99cd53..343bcf265c 100644 --- a/.agents/skills/align-recipe-pyproject/scripts/align_pyproject.py +++ b/.agents/skills/align-recipe-pyproject/scripts/align_pyproject.py @@ -1015,7 +1015,8 @@ def run( ERROR, f"{recipe_dir} looks like a git repository root (it contains " f"a .git entry), not a recipe. Point --recipe-dir at a recipe " - f"root under core/python//, contrib//, or skills/python//.", + f"root under core/python//, contrib/python//, or " + f"skills///.", ) ) return report diff --git a/.agents/skills/extract-python-environment-variables/SKILL.md b/.agents/skills/extract-python-environment-variables/SKILL.md index b31f3bbcc6..9a9705cfd1 100644 --- a/.agents/skills/extract-python-environment-variables/SKILL.md +++ b/.agents/skills/extract-python-environment-variables/SKILL.md @@ -219,7 +219,7 @@ Python source (unchanged behaviour) and WRITES only to `.env.example`. | Field | Required | Description | |-------|----------|-------------| -| Recipe directory | Yes | Path to the recipe root (e.g. `contrib/my-recipe`, `core/python/my-recipe`, or `skills/python/my-skill`) | +| Recipe directory | Yes | Path to the recipe root (e.g. `contrib/python/my-recipe`, `core/python/my-recipe`, or `skills/retail/store-ops`) | If the user has not specified the recipe directory, ask for it before proceeding. diff --git a/.agents/skills/extract-python-environment-variables/scripts/extract_env_vars.py b/.agents/skills/extract-python-environment-variables/scripts/extract_env_vars.py index d4955e8b85..ce9f0346da 100644 --- a/.agents/skills/extract-python-environment-variables/scripts/extract_env_vars.py +++ b/.agents/skills/extract-python-environment-variables/scripts/extract_env_vars.py @@ -2176,7 +2176,7 @@ def main() -> None: parser.add_argument( "--recipe-dir", required=True, - help="Path to the root of the Python recipe (e.g. contrib/my-recipe)", + help="Path to the root of the Python recipe (e.g. contrib/python/my-recipe)", ) parser.add_argument( "--dry-run", diff --git a/.agents/skills/generate-manifest/SKILL.md b/.agents/skills/generate-manifest/SKILL.md index d7682be1f2..cd737dfe73 100644 --- a/.agents/skills/generate-manifest/SKILL.md +++ b/.agents/skills/generate-manifest/SKILL.md @@ -11,7 +11,7 @@ Scan a recipe directory and produce a valid `manifest.yaml` for it, then validat ### 1. Identify the target recipe -The user will provide a path to a recipe directory (e.g. `core/rag-agent-search`, `contrib/my-recipe`, `skills/python/my-skill`). If they don't, ask for it before proceeding. +The user will provide a path to a recipe directory (e.g. `core/python/rag-agent-search`, `contrib/python/my-recipe`, `skills/retail/store-ops`). If they don't, ask for it before proceeding. ### 2. Read the schema @@ -101,7 +101,7 @@ After writing the manifest, run the validator **from the repo root** (do not `cd uv run validate manifest ``` -`` must be **relative to the repo root** (e.g. `core/python/my-recipe`, `contrib/my-recipe`, `skills/python/my-skill`), not an absolute path. +`` must be **relative to the repo root** (e.g. `core/python/my-recipe`, `contrib/python/my-recipe`, `skills/retail/store-ops`), not an absolute path. Interpret the result carefully — there are two kinds of failure: diff --git a/.agents/skills/generate-python-runnability-test/SKILL.md b/.agents/skills/generate-python-runnability-test/SKILL.md index e45c333c4d..c7cd88e1f0 100644 --- a/.agents/skills/generate-python-runnability-test/SKILL.md +++ b/.agents/skills/generate-python-runnability-test/SKILL.md @@ -20,7 +20,7 @@ metadata: # Generate Python Runnability Test -Use this skill to create the `tests/test_runnability.py` file that every Python recipe under `core/python/`, `contrib/`, or `skills/python/` must ship (see `python-validate-recipe.yml` Check 4). The generated test is deliberately minimal — it just verifies the agent module imports and defines the expected globals. Business-logic testing lives elsewhere. +Use this skill to create the `tests/test_runnability.py` file that every Python recipe under `core/python/`, `contrib/python/`, or `skills///` must ship (see `python-validate-recipe.yml` Check 4). The generated test is deliberately minimal — it just verifies the agent module imports and defines the expected globals. Business-logic testing lives elsewhere. --- @@ -60,7 +60,7 @@ Runs `scripts/generate_runnability_test.py` against a recipe directory. Steps: 1. **Always use the script — never hand-write `tests/test_runnability.py` yourself.** The skill exists to keep the boilerplate consistent across recipes. -2. **Ask for the recipe directory** if the user hasn't given one. Recipe roots live under `core/python//`, `contrib//`, or `skills/python//`. +2. **Ask for the recipe directory** if the user hasn't given one. Recipe roots live under `core/python//`, `contrib/python//`, or `skills///`. 3. **Always start with `--dry-run`** unless the user has explicitly said "apply", "generate it", "just do it", or equivalent. Show them what would land before writing. @@ -86,7 +86,7 @@ Runs `scripts/generate_runnability_test.py` against a recipe directory. Steps: | Field | Required | Description | |---|---|---| -| `--recipe-dir` | Yes | Path to the recipe root (e.g. `core/python/cross-session-memory`, `contrib/my-recipe`, `skills/python/my-skill`). | +| `--recipe-dir` | Yes | Path to the recipe root (e.g. `core/python/cross-session-memory`, `contrib/python/my-recipe`, `skills/retail/store-ops`). | | `--dry-run` | No | Print the JSON report (with the generated content in `test_content`) without writing any file. | | `--overwrite` | No | Overwrite an existing `tests/test_runnability.py`. Default: refuse and exit 1. | | `--agent-file` | No | Override auto-detection of the entry-point file. Path is relative to `--recipe-dir` (or absolute). Use when the recipe uses a non-standard layout (rare — <2% of recipes). | diff --git a/.agents/skills/prepare-python-recipe/SKILL.md b/.agents/skills/prepare-python-recipe/SKILL.md index 629a3580a7..c60dbbf304 100644 --- a/.agents/skills/prepare-python-recipe/SKILL.md +++ b/.agents/skills/prepare-python-recipe/SKILL.md @@ -2,8 +2,9 @@ name: prepare-python-recipe description: > End-to-end orchestration to prepare or update a Python recipe under - core/python/, contrib/, or skills/python/ so it passes every check in - .github/workflows/python-validate-recipe.yml. Runs seven phases in + core/python/, contrib/python/, or skills/// so it + passes every check in .github/workflows/python-validate-recipe.yml. + Runs seven phases in order on an already-in-place recipe: manifest.yaml generation, environment-variable extraction, pyproject.toml alignment, ruff format+check, per-recipe `uv lock`, runnability-test generation, and a @@ -55,7 +56,7 @@ The skill assumes the user has already: 1. **Deactivated** any active Python virtual environment. 2. **Pulled latest** from `origin` (`git pull` at the repo root). 3. **Synced repo root deps** (`uv sync` at the repo root). -4. **Placed the recipe at its target path** — either freshly scaffolded, moved from another location, or renamed to its final basename under `core/python//`, `contrib//`, or `skills/python//`. +4. **Placed the recipe at its target path** — either freshly scaffolded, moved from another location, or renamed to its final basename under `core/python//`, `contrib/python//`, or `skills///`. 5. **Committed the original recipe** so `git diff` shows what the skill changed. If the user has NOT done these and asks you to run the skill anyway, tell them to complete the prerequisites first and stop. Do NOT run `git pull`, `git commit`, deactivate their venv, or move/rename directories on their behalf — those are deliberately out of scope. @@ -124,7 +125,7 @@ At the end, print a summary table and remind the user to `git diff` and commit | Field | Required | Description | |---|---|---| -| Recipe directory | Yes | Path to the recipe root (e.g. `core/python/cross-session-memory`, `contrib/my-recipe`, `skills/python/my-skill`). Passed to every sub-script as `--recipe-dir`. | +| Recipe directory | Yes | Path to the recipe root (e.g. `core/python/cross-session-memory`, `contrib/python/my-recipe`, `skills/retail/store-ops`). Passed to every sub-script as `--recipe-dir`. | If the user has not specified the recipe directory, ask for it before proceeding. diff --git a/.agents/skills/scaffold-python-recipe/SKILL.md b/.agents/skills/scaffold-python-recipe/SKILL.md index f90e8c3972..9eaae0a8f9 100644 --- a/.agents/skills/scaffold-python-recipe/SKILL.md +++ b/.agents/skills/scaffold-python-recipe/SKILL.md @@ -31,12 +31,15 @@ You **must** have both pieces of information below before running the script. If ### 1. Output Directory (Required — must ask if not provided) -The user must choose one of these two valid locations: -- `contrib/` -- `core/python/` +The user must choose one of these valid locations: +- `contrib/python/` — community recipes; preferred for new Python recipes +- `core/python/` — curated recipes +- `contrib/` — legacy flat layout, still accepted; prefer `contrib/python/` If the user has not specified which directory, ask them to choose. Do not proceed until a valid choice is confirmed. +This skill **cannot** scaffold a vertical skill (`skills///`). Those have a different shape — `SKILL.md`, `EVAL.yaml`, `scripts/`, `assets/`, `references/`, `tests/unit/` — and no template ships for them yet. If the user asks for one, tell them so rather than scaffolding into the wrong place. + ### 2. Recipe Name (Required — must ask if not provided) The recipe name must satisfy **all** of the following rules: diff --git a/.agents/skills/scaffold-python-recipe/scripts/scaffold.py b/.agents/skills/scaffold-python-recipe/scripts/scaffold.py index 78a2d6bfed..672adec034 100644 --- a/.agents/skills/scaffold-python-recipe/scripts/scaffold.py +++ b/.agents/skills/scaffold-python-recipe/scripts/scaffold.py @@ -68,12 +68,32 @@ def is_safe_output_dir(output_dir: str) -> bool: Blocks relative escapes like ``../../other-repo`` for any caller. Absolute destinations are allowed here so the function stays usable as a testable - primitive; the CLI additionally restricts ``--output-dir`` to the two - documented locations (see ``__main__``). + primitive; the CLI additionally restricts ``--output-dir`` to the + documented locations (see ``ALLOWED_OUTPUT_DIRS``). """ return ".." not in re.split(r"[\\/]", output_dir) +# Locations the CLI will scaffold into. `contrib` (flat) stays accepted +# because recipes already live there, but `contrib/python` is the documented +# home for new Python recipes. +# +# Vertical skills (skills//) are deliberately absent: +# they need a different template — SKILL.md, EVAL.yaml, scripts/, assets/, +# references/, tests/unit/ — which this skill does not ship. Refusing is +# better than scaffolding a recipe-shaped tree into a skills path. +ALLOWED_OUTPUT_DIRS = ("contrib", "contrib/python", "core/python") + + +def is_allowed_output_dir(output_dir: str) -> bool: + """Return True if ``output_dir`` is one of the documented locations. + + Trailing slashes are tolerated to match the paths shown in the skill's + instructions. + """ + return output_dir.rstrip("/") in ALLOWED_OUTPUT_DIRS + + def replace_in_file(filepath: str, replacements: dict[str, str]) -> None: """Reads a file, replaces placeholder tokens, and writes it back.""" with open(filepath, encoding="utf-8") as f: @@ -175,13 +195,11 @@ def scaffold( # Enforce the documented, valid locations (SKILL.md) at the CLI so a direct # invocation can't scaffold outside the repo (e.g. --output-dir - # ../../other-repo) or into an unintended location. Trailing slashes are - # tolerated to match the paths shown in the skill's instructions. - allowed_output_dirs = ("contrib", "core/python") - if args.output_dir.rstrip("/") not in allowed_output_dirs: + # ../../other-repo) or into an unintended location. + if not is_allowed_output_dir(args.output_dir): print( "Error: --output-dir must be one of " - f"{allowed_output_dirs} (got '{args.output_dir}')." + f"{ALLOWED_OUTPUT_DIRS} (got '{args.output_dir}')." ) sys.exit(2) diff --git a/.agents/skills/scaffold-python-recipe/tests/test_scaffold.py b/.agents/skills/scaffold-python-recipe/tests/test_scaffold.py index 691ed30d1d..a51b6014b6 100644 --- a/.agents/skills/scaffold-python-recipe/tests/test_scaffold.py +++ b/.agents/skills/scaffold-python-recipe/tests/test_scaffold.py @@ -272,3 +272,56 @@ def test_scaffold_omits_cache_junk(tmp_path, templates_with_junk): if p.name in {".ruff_cache", "__pycache__"} or p.suffix == ".pyc" ] assert leaked == [] + + +# --------------------------------------------------------------------------- +# is_allowed_output_dir / CLI --output-dir allow-list +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "output_dir", + [ + "contrib", + "contrib/python", + "core/python", + "contrib/python/", + "core/python/", + ], +) +def test_allows_documented_output_dirs(output_dir): + """contrib/python is the documented home for new Python recipes. It was + previously rejected, so scaffolding into it was impossible.""" + assert m.is_allowed_output_dir(output_dir) is True + + +@pytest.mark.parametrize( + "output_dir", + [ + # Vertical skills need a template this skill does not ship — SKILL.md, + # EVAL.yaml, scripts/, assets/, references/, tests/unit/ — so they must + # be refused rather than filled with a recipe-shaped tree. + "skills", + "skills/retail", + "skills/retail/store-ops", + # Retired roots and other stray locations. + "python/agents", + "core", + "docs", + "../../other-repo", + "", + ], +) +def test_rejects_everything_else(output_dir): + assert m.is_allowed_output_dir(output_dir) is False + + +def test_allow_list_matches_the_skill_docs(): + """SKILL.md tells the assistant which locations to offer; drift between + the prose and the check would send contributors down a dead end.""" + skill_md = (Path(__file__).resolve().parents[1] / "SKILL.md").read_text( + encoding="utf-8" + ) + for allowed in m.ALLOWED_OUTPUT_DIRS: + assert f"`{allowed}/`" in skill_md, allowed + assert "skills///" in skill_md diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 51aac691bb..8961d3777b 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -27,3 +27,6 @@ /contrib/java/** @happyhuman /contrib/typescript/** @happyhuman /contrib/kotlin/** @happyhuman + +# --- skills/ --- +/skills/** @happyhuman diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 5979c0d4ab..2164635f8e 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -1,12 +1,15 @@ -# AUTO-GENERATED — do not edit by hand. -# Owned by .github/scripts/generate_dependabot.py +# GENERATED FILE — every line below is written by +# .github/scripts/generate_dependabot.py. Do not edit it by hand. # -# This file is regenerated automatically on every push to main that touches -# core/, contrib/, or skills/. To force an immediate refresh, trigger the -# "Sync Dependabot Config" workflow manually from the Actions tab, or run: +# Nothing triggers that script for you — there is no push hook, schedule, or +# bot that keeps this in sync. Whoever adds or removes a recipe has to run it +# and commit the result in the same pull request: # # python .github/scripts/generate_dependabot.py # +# CI enforces this — the "Check dependabot.yml is up to date" job fails the +# PR if this file no longer matches the recipe tree. +# # Ecosystems tracked: uv (Python), gomod (Go), maven (Java), # gradle (Kotlin), npm (TypeScript/JS), github-actions @@ -58,6 +61,29 @@ updates: patterns: - "*" + - package-ecosystem: "uv" + directory: "/contrib/python/market-research-agent" + schedule: + interval: weekly + day: monday + time: "02:00" + commit-message: + prefix: chore + include: scope + labels: + - dependencies + open-pull-requests-limit: 1 + # Wait a few days after a release before opening a PR, so the community + # catches broken releases before we auto-merge them. Security updates + # bypass this cooldown and fire immediately. + cooldown: + default-days: 7 + semver-major-days: 14 + groups: + all-dependencies: + patterns: + - "*" + - package-ecosystem: "uv" directory: "/core/python/ambient-expense-agent" schedule: diff --git a/.github/policy.yml b/.github/policy.yml index c2f9bb8daf..0858e7b62c 100644 --- a/.github/policy.yml +++ b/.github/policy.yml @@ -19,8 +19,15 @@ # Recipes live under core// or contrib// # (e.g. core/python/rag-agent-search, contrib/java/hello-agent). # +# skills/ uses a VERTICAL rather than a language for its middle component: +# skills// (e.g. skills/retail/store-ops). The vertical +# surfaces ownership, so a team can see its whole surface at a glance, and +# it is mandatory — tools/validate_placement.py rejects a solution placed +# directly under skills/. +# # The applicable tier is chosen by: -# 1. Top-level ancestor: core → `core` limits; contrib → `contrib` limits. +# 1. Top-level ancestor: core → `core` limits; contrib → `contrib` +# limits; skills → `skills` limits. # 2. `large:` field in the recipe's manifest.yaml (default: false): # false / omitted → `default` limits; true → `large` limits. # @@ -45,6 +52,16 @@ recipe_size_limits: large: max_files: 200 max_size_mb: 10 + # skills/ deliberately mirrors contrib: a vertical skill is a focused + # solution (SKILL.md + scripts + design spec + eval), not a full + # application, so the tighter tier is the right default. + skills: + default: + max_files: 70 + max_size_mb: 2 + large: + max_files: 200 + max_size_mb: 10 # -------------------------------------------------------------------------- # Recipe naming — enforced by @@ -58,6 +75,30 @@ recipe_size_limits: recipe_naming: max_folder_name_length: 30 +# -------------------------------------------------------------------------- +# Retired recipe roots — enforced by the "Check for work in retired folders" +# job in .github/workflows/validate-recipe-structure.yml via +# tools/check_frozen_paths.py. +# +# Recipes used to live at /agents/ in the repo root. Those +# roots are closed: both new recipes and changes to existing ones belong in +# contrib//. A pull request that ADDS or MODIFIES any file +# under a path listed here fails. Deletions and renames are exempt, so +# migrating a recipe out of a retired folder is never blocked. +# +# Scoped to /agents rather than so that the deprecation +# notice in each /README.md stays editable. +# +# Escape hatches: the job skips for bot authors, and for any PR carrying the +# `allow-legacy-path` label (only users with write access can apply labels). +# -------------------------------------------------------------------------- +frozen_paths: + - python/agents + - java/agents + - go/agents + - kotlin/agents + - typescript/agents + # -------------------------------------------------------------------------- # Excluded paths — files/directories that never count toward size or file # count when validating a recipe. @@ -182,13 +223,22 @@ required_files: core: - AGENTS.md contrib: [] - # skills/ is a planned recipe root. SKILL.md is confirmed; other - # requirements are TBD and will be added here as they're defined. - # No entry under recipe_size_limits: the structure checker treats a - # root with no configured size limits as "size checks skipped" — safe - # for a root whose expected shape we don't yet want to constrain. + # Vertical skills live at skills//. + # SKILL.md — the conversational installer: runs the interview, + # writes the design spec, performs setup. + # EVAL.yaml — self-verifying proof of performance (eval rubrics). + # The directories a vertical skill must also ship (scripts/, assets/, + # references/, tests/unit/) are listed under `required_dirs` below — + # this section can only express files. + # + # A Python vertical skill additionally picks up + # required_files.by_language.python (pyproject.toml, uv.lock, + # .env.example, tests/test_runnability.py) via manifest.language. The + # path does not encode the language for this root — the middle folder + # is a vertical — so the manifest is what resolves it. skills: - SKILL.md + - EVAL.yaml by_language: python: @@ -202,3 +252,64 @@ required_files: go: [] kotlin: [] typescript: [] + +# -------------------------------------------------------------------------- +# Required directories — enforced by +# .github/workflows/validate-recipe-structure.yml via +# tools/validate_structure.py (check 6). +# +# Resolved exactly like required_files above: the UNION of `always`, +# `by_root[]`, and `by_language[]`. It is a +# separate section because directories need a different existence test — +# a `scripts` entry under required_files could never be satisfied, since +# that check requires a file. +# +# An empty directory PASSES. assets/ and references/ are legitimately +# empty for some skills, and git cannot commit an empty directory, so in +# practice they carry a .gitkeep. +# -------------------------------------------------------------------------- +required_dirs: + always: [] + + by_root: + core: [] + contrib: [] + # The shape every vertical skill ships: + # scripts/ — functional code: ADK agents, model implementations, + # one-shot GCP setup scripts. + # assets/ — demo data, e.g. sample catalogs. + # references/ — deep-dive documentation for human review. + # tests/unit/ — unit tests, for reliability. + skills: + - scripts + - assets + - references + - tests/unit + + by_language: + python: [] + java: [] + go: [] + kotlin: [] + typescript: [] + +# -------------------------------------------------------------------------- +# Case-insensitive required entries. +# +# Names in required_files / required_dirs are matched EXACTLY by default, +# compared in Python rather than by asking the filesystem — otherwise a +# contributor on macOS (case-insensitive by default) could name a file +# `PyProject.toml`, watch it pass locally, and fail on Linux CI. +# +# Listing an entry here relaxes that for it alone: any spelling is +# accepted, and the checker prints an advisory naming the canonical form. +# +# Add to this list SPARINGLY. Only files that nothing else parses belong +# here. Never add pyproject.toml or uv.lock (uv resolves them by exact +# name), tests/test_runnability.py (pytest collects it), or manifest.yaml +# (globbed by the recipe tooling) — accepting a variant spelling would +# pass this check and then break the tool that actually reads the file, +# which is a worse failure than the one the leniency prevents. +# -------------------------------------------------------------------------- +case_insensitive_files: + - EVAL.yaml diff --git a/.github/scripts/generate_dependabot.py b/.github/scripts/generate_dependabot.py index 77629d354a..a6b615a3fc 100644 --- a/.github/scripts/generate_dependabot.py +++ b/.github/scripts/generate_dependabot.py @@ -19,8 +19,17 @@ python .github/scripts/generate_dependabot.py # writes in-place python .github/scripts/generate_dependabot.py --check # exit 1 if stale -Invoked automatically by .github/workflows/sync-dependabot-config.yml on -every push to main that touches core/, contrib/, or skills/. +Who runs this +------------- +There is no automatic regeneration on push. main is protected, so the +github-actions bot cannot commit to it. Instead: + + 1. You add or remove a recipe — run this script and commit the updated + .github/dependabot.yml in the SAME pull request. The freshness gate in + .github/workflows/validate-recipe-structure.yml fails your PR otherwise. + 2. A maintainer can run the "Sync Dependabot Config" workflow manually from + the Actions tab to catch any drift that slipped through; it opens a small + PR with the regenerated file. DO NOT edit .github/dependabot.yml by hand — this script owns it. """ @@ -28,6 +37,7 @@ from __future__ import annotations import argparse +import difflib import json import os import sys @@ -45,8 +55,15 @@ # Directory names that are never recipe roots — skip entirely during the walk SKIP_DIRS = { - ".venv", "node_modules", ".gradle", ".git", - "__pycache__", ".tox", ".mypy_cache", "dist", "build", + ".venv", + "node_modules", + ".gradle", + ".git", + "__pycache__", + ".tox", + ".mypy_cache", + "dist", + "build", } # --------------------------------------------------------------------------- @@ -69,7 +86,9 @@ def _is_maven(d: Path) -> bool: def _is_gradle(d: Path) -> bool: # Only the root project has settings.gradle.kts; sub-modules do not. - return (d / "build.gradle.kts").is_file() and (d / "settings.gradle.kts").is_file() + return (d / "build.gradle.kts").is_file() and ( + d / "settings.gradle.kts" + ).is_file() def _is_npm(d: Path) -> bool: @@ -84,11 +103,11 @@ def _is_npm(d: Path) -> bool: DETECTORS: list[tuple[str, object]] = [ - ("uv", _is_uv), - ("gomod", _is_gomod), - ("maven", _is_maven), + ("uv", _is_uv), + ("gomod", _is_gomod), + ("maven", _is_maven), ("gradle", _is_gradle), - ("npm", _is_npm), + ("npm", _is_npm), ] # --------------------------------------------------------------------------- @@ -122,23 +141,30 @@ def scan(repo_root: Path) -> list[dict]: continue seen.add(key) - entries.append({"package-ecosystem": ecosystem, "directory": rel}) + entries.append( + {"package-ecosystem": ecosystem, "directory": rel} + ) entries.sort(key=lambda e: (e["package-ecosystem"], e["directory"])) return entries + # --------------------------------------------------------------------------- # YAML renderer # Keeps zero runtime dependencies — uses only stdlib. # --------------------------------------------------------------------------- -def _render_entry(ecosystem: str, directory: str, *, extra_labels: list[str] | None = None) -> str: +def _render_entry( + ecosystem: str, directory: str, *, extra_labels: list[str] | None = None +) -> str: labels_lines = " - dependencies" if extra_labels: labels_lines += "\n" + "\n".join(f" - {lb}" for lb in extra_labels) - group_name = "all-actions" if ecosystem == "github-actions" else "all-dependencies" + group_name = ( + "all-actions" if ecosystem == "github-actions" else "all-dependencies" + ) # `semver-major-days` is only valid for ecosystems that follow semver. # github-actions releases (v1, v2, ...) don't, and Dependabot rejects the @@ -175,15 +201,18 @@ def _render_entry(ecosystem: str, directory: str, *, extra_labels: list[str] | N def render(entries: list[dict]) -> str: header = """\ -# AUTO-GENERATED — do not edit by hand. -# Owned by .github/scripts/generate_dependabot.py +# GENERATED FILE — every line below is written by +# .github/scripts/generate_dependabot.py. Do not edit it by hand. # -# This file is regenerated automatically on every push to main that touches -# core/, contrib/, or skills/. To force an immediate refresh, trigger the -# "Sync Dependabot Config" workflow manually from the Actions tab, or run: +# Nothing triggers that script for you — there is no push hook, schedule, or +# bot that keeps this in sync. Whoever adds or removes a recipe has to run it +# and commit the result in the same pull request: # # python .github/scripts/generate_dependabot.py # +# CI enforces this — the "Check dependabot.yml is up to date" job fails the +# PR if this file no longer matches the recipe tree. +# # Ecosystems tracked: uv (Python), gomod (Go), maven (Java), # gradle (Kotlin), npm (TypeScript/JS), github-actions @@ -191,8 +220,7 @@ def render(entries: list[dict]) -> str: updates: """ body_parts = [ - _render_entry(e["package-ecosystem"], e["directory"]) - for e in entries + _render_entry(e["package-ecosystem"], e["directory"]) for e in entries ] # Static github-actions entry is always last body_parts.append( @@ -200,13 +228,16 @@ def render(entries: list[dict]) -> str: ) return header + "\n".join(body_parts) + # --------------------------------------------------------------------------- # Main # --------------------------------------------------------------------------- def main(argv: list[str] | None = None) -> int: - parser = argparse.ArgumentParser(description="Generate .github/dependabot.yml") + parser = argparse.ArgumentParser( + description="Generate .github/dependabot.yml" + ) parser.add_argument( "--check", action="store_true", @@ -217,19 +248,37 @@ def main(argv: list[str] | None = None) -> int: entries = scan(REPO_ROOT) content = render(entries) - current = OUTPUT_FILE.read_text(encoding="utf-8") if OUTPUT_FILE.is_file() else "" + current = ( + OUTPUT_FILE.read_text(encoding="utf-8") if OUTPUT_FILE.is_file() else "" + ) if content == current: print(f"dependabot.yml is up to date ({len(entries)} recipe entries).") return 0 if args.check: - print("dependabot.yml is STALE. Run the script locally to regenerate:") + print("dependabot.yml is STALE — it does not match the recipe tree.") + print("") + print("Fix it by running this from the repo root and committing the") + print("result in this same pull request:") + print("") print(" python .github/scripts/generate_dependabot.py") + print("") + print("Missing (+) and stale (-) lines:") + print("") + diff = difflib.unified_diff( + current.splitlines(keepends=True), + content.splitlines(keepends=True), + fromfile="committed .github/dependabot.yml", + tofile="expected from the current recipe tree", + ) + sys.stdout.writelines(diff) return 1 OUTPUT_FILE.write_text(content, encoding="utf-8") - print(f"dependabot.yml written: {len(entries)} recipe entries + github-actions.") + print( + f"dependabot.yml written: {len(entries)} recipe entries + github-actions." + ) for e in entries: print(f" [{e['package-ecosystem']:8s}] {e['directory']}") return 0 diff --git a/.github/workflows/python-dependency-policy.yml b/.github/workflows/python-dependency-policy.yml index 63949d0df2..f0e5cc4cf0 100644 --- a/.github/workflows/python-dependency-policy.yml +++ b/.github/workflows/python-dependency-policy.yml @@ -48,6 +48,20 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 15 + # Single source of truth for the recipe roots this workflow scans. + # Mirrors tools/validate_manifest.py:RECIPE_ROOTS and the lists in + # python-tests.yml:85 / python-validate-recipe.yml:129. Both matcher + # forms used below (the `grep -E` alternation and the `find` argument + # list) are derived from this one value. + # + # Deriving rather than duplicating is deliberate: this list was + # previously hardcoded at six separate sites, and when skills/ was + # added to `on.paths` above, those six were missed. The result was a + # workflow that triggered on skills/ lockfiles and then validated + # nothing while still reporting success. + env: + RECIPE_ROOTS: "core contrib skills" + steps: - name: Checkout repository uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # ratchet:actions/checkout@v4 @@ -70,11 +84,12 @@ jobs: # On pull_request: only lockfiles that were added/modified in this PR, plus # the sibling uv.lock for any pyproject.toml that was changed. The root # uv.lock is included if the root uv.lock or pyproject.toml was touched. - # On push to main or workflow_dispatch: all uv.lock files under core/ and - # contrib/, plus the root uv.lock (full scan as a safety net). + # On push to main or workflow_dispatch: all uv.lock files under the + # recipe roots, plus the root uv.lock (full scan as a safety net). # - # Samples live exclusively under core/ and contrib/. The root uv.lock is - # treated separately as repo-level infrastructure. + # Recipes live exclusively under the recipe roots (see RECIPE_ROOTS on + # the job above). The root uv.lock is treated separately as repo-level + # infrastructure. # # The resolved list is written to $LOCKFILES_FILE (one path per line) so # that subsequent steps can iterate with `while IFS= read -r` and avoid @@ -93,23 +108,38 @@ jobs: LOCKFILES_FILE="${RUNNER_TEMP}/lockfiles.txt" + # Derive both matcher forms from the single RECIPE_ROOTS value. + ROOTS_RE="${RECIPE_ROOTS// /|}" # core|contrib|skills + read -ra ROOTS <<< "$RECIPE_ROOTS" + + # Only hand `find` roots that actually exist. find errors on a + # missing starting-point, and that error would otherwise have to be + # suppressed with 2>/dev/null — which silently turns "this root is + # missing" into "this root has no lockfiles", a false-clean result. + # Filtering up front means the suppression is unnecessary and a + # genuine find failure stays visible. + EXISTING_ROOTS=() + for root in "${ROOTS[@]}"; do + [ -d "$root" ] && EXISTING_ROOTS+=("$root") + done + if [ "$EVENT_NAME" = "pull_request" ]; then git fetch origin "$BASE_REF" # --diff-filter=d: exclude deleted files; downstream [ -f ] guards # are kept as defence-in-depth but the filter makes intent explicit. CHANGED=$(git diff --name-only --diff-filter=d "origin/$BASE_REF"...HEAD) - # Collect directly changed uv.lock files under core/ or contrib/ - LOCKS=$(echo "$CHANGED" | grep -E "^(core|contrib)/.*uv\.lock$" || true) + # Collect directly changed uv.lock files under any recipe root + LOCKS=$(echo "$CHANGED" | grep -E "^(${ROOTS_RE})/.*uv\.lock$" || true) - # For any changed pyproject.toml under core/ or contrib/, add its sibling uv.lock + # For any changed pyproject.toml under a recipe root, add its sibling uv.lock while IFS= read -r pyproject; do [ -z "$pyproject" ] && continue sibling="$(dirname "$pyproject")/uv.lock" if [ -f "$sibling" ]; then LOCKS=$(printf "%s\n%s" "$LOCKS" "$sibling") fi - done <<< "$(echo "$CHANGED" | grep -E "^(core|contrib)/.*pyproject\.toml$" || true)" + done <<< "$(echo "$CHANGED" | grep -E "^(${ROOTS_RE})/.*pyproject\.toml$" || true)" # Include the root uv.lock if the root uv.lock or pyproject.toml was touched ROOT_CHANGED=$(echo "$CHANGED" | grep -E "^(uv\.lock|pyproject\.toml)$" || true) @@ -125,10 +155,11 @@ jobs: INFRA_CHANGED=$(echo "$CHANGED" | grep -E "^(\.github/workflows/python-dependency-policy\.yml|\.github/scripts/check_lockfile_hashes\.py)$" || true) if [ -n "$INFRA_CHANGED" ]; then echo "Workflow or script changed — falling back to full scan." - # Use { } group rather than `find uv.lock core contrib …` to avoid - # passing a potentially-absent root file as a find starting-point, - # which would error and be silently swallowed by 2>/dev/null. - LOCKS=$({ [ -f uv.lock ] && echo uv.lock; find core contrib -name "uv.lock" 2>/dev/null; } || true) + # Use { } group rather than `find uv.lock …` to avoid + # passing a potentially-absent root FILE as a find starting-point. + # EXISTING_ROOTS (built above) covers the same hazard for the + # root DIRECTORIES, so no 2>/dev/null suppression is needed here. + LOCKS=$({ [ -f uv.lock ] && echo uv.lock; [ ${#EXISTING_ROOTS[@]} -gt 0 ] && find "${EXISTING_ROOTS[@]}" -name "uv.lock"; } || true) fi # Deduplicate and filter to only existing files @@ -137,7 +168,7 @@ jobs: done > "$LOCKFILES_FILE" || true else # push to main or workflow_dispatch — scan everything - { [ -f uv.lock ] && echo uv.lock; find core contrib -name "uv.lock" 2>/dev/null; } > "$LOCKFILES_FILE" || true + { [ -f uv.lock ] && echo uv.lock; [ ${#EXISTING_ROOTS[@]} -gt 0 ] && find "${EXISTING_ROOTS[@]}" -name "uv.lock"; } > "$LOCKFILES_FILE" || true fi if [ ! -s "$LOCKFILES_FILE" ]; then @@ -356,10 +387,11 @@ jobs: echo "[PASS] All lockfiles are up-to-date." # --------------------------------------------------------------------------- - # Check 6: Every pyproject.toml (root, core/, contrib/) must have a sibling - # uv.lock. Only checked for pyproject.toml files touched in this PR (or all - # of them on push/dispatch). Skips config-only pyproject.toml files that have - # no [project] table and no [tool.uv] section. + # Check 6: Every pyproject.toml (the root one, plus any under a recipe + # root — see RECIPE_ROOTS on the job) must have a sibling uv.lock. Only + # checked for pyproject.toml files touched in this PR (or all of them on + # push/dispatch). Skips config-only pyproject.toml files that have no + # [project] table and no [tool.uv] section. # --------------------------------------------------------------------------- - name: Check every pyproject.toml has a lockfile env: @@ -368,17 +400,28 @@ jobs: run: | set -euo pipefail + # Same derivation as the Resolve step above: shell variables do not + # carry across steps, but the RECIPE_ROOTS value they derive from is + # job-level, so the root list is still defined in exactly one place. + ROOTS_RE="${RECIPE_ROOTS// /|}" + read -ra ROOTS <<< "$RECIPE_ROOTS" + + EXISTING_ROOTS=() + for root in "${ROOTS[@]}"; do + [ -d "$root" ] && EXISTING_ROOTS+=("$root") + done + # On pull_request the base ref was already fetched in the Resolve step; # no second git fetch needed. if [ "$EVENT_NAME" = "pull_request" ]; then CHANGED=$(git diff --name-only --diff-filter=d "origin/$BASE_REF"...HEAD) - # Include root pyproject.toml and any under core/ or contrib/ - PYPROJECTS=$(echo "$CHANGED" | grep -E "^(pyproject\.toml|(core|contrib)/.*pyproject\.toml)$" || true) + # Include the root pyproject.toml and any under a recipe root + PYPROJECTS=$(echo "$CHANGED" | grep -E "^(pyproject\.toml|(${ROOTS_RE})/.*pyproject\.toml)$" || true) else # { } group avoids passing a potentially-absent root pyproject.toml as - # a find starting-point (find would error; 2>/dev/null would silently - # swallow that, producing a false-clean result). - PYPROJECTS=$({ [ -f pyproject.toml ] && echo pyproject.toml; find core contrib -name "pyproject.toml" 2>/dev/null; } || true) + # a find starting-point (find would error). EXISTING_ROOTS covers the + # same hazard for the root directories, so no 2>/dev/null is needed. + PYPROJECTS=$({ [ -f pyproject.toml ] && echo pyproject.toml; [ ${#EXISTING_ROOTS[@]} -gt 0 ] && find "${EXISTING_ROOTS[@]}" -name "pyproject.toml"; } || true) fi if [ -z "$PYPROJECTS" ]; then diff --git a/.github/workflows/python-validate-recipe.yml b/.github/workflows/python-validate-recipe.yml index 6eabfba326..d192a63830 100644 --- a/.github/workflows/python-validate-recipe.yml +++ b/.github/workflows/python-validate-recipe.yml @@ -34,12 +34,12 @@ jobs: steps: - name: Checkout repository - uses: actions/checkout@v4 + uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # ratchet:actions/checkout@v4 with: fetch-depth: 0 - name: Install uv - uses: astral-sh/setup-uv@v6 + uses: astral-sh/setup-uv@445689ea25e0de0a23313031f5fe577c74ae45a1 # ratchet:astral-sh/setup-uv@v6.3.0 with: python-version: "3.11" enable-cache: false @@ -119,10 +119,11 @@ jobs: # the diff step uses — with --language python. This keeps one # source of truth for the layout matrix (path → recipe → # language) instead of re-implementing it in shell. `skills/` - # is included so that a Python recipe under skills/python/… - # (or a flat skills// with language: python in its - # manifest) is picked up by the full scan; missing roots are - # tolerated silently via 2>/dev/null. + # is included so that a Python skill at + # skills/// is picked up by the full scan. + # Its language comes from manifest.language rather than the path: + # the middle component under skills/ is a vertical (retail/, hr/), + # not a language. Missing roots are tolerated via 2>/dev/null. # NOT `--no-project`: see the notes on the sibling invocation # above. ALL=$(find core contrib skills -type f 2>/dev/null | uv run python tools/affected_recipes.py --language python) diff --git a/.github/workflows/sync-dependabot-config.yml b/.github/workflows/sync-dependabot-config.yml index 85e6f3b7b9..5175e789b7 100644 --- a/.github/workflows/sync-dependabot-config.yml +++ b/.github/workflows/sync-dependabot-config.yml @@ -15,10 +15,12 @@ name: Sync Dependabot Config # 2. A maintainer (or CI reminder) runs this workflow manually. # 3. This workflow opens a small sync PR that a code owner approves. # -# The PR-time freshness check (.github/workflows/validate-recipe-structure.yml -# or any workflow that calls generate_dependabot.py --check) will fail the -# recipe PR itself if the contributor forgot to regenerate the file, giving -# an earlier signal before this manual step is needed. +# The PR-time freshness check — the "Check dependabot.yml is up to date" job +# in .github/workflows/validate-recipe-structure.yml — fails the recipe PR +# itself if the contributor forgot to regenerate the file, giving an earlier +# signal before this manual step is needed. That job is what actually keeps +# the file in sync today; this workflow has not yet been exercised, so verify +# it end to end before relying on it. on: workflow_dispatch: diff --git a/.github/workflows/tools-tests.yml b/.github/workflows/tools-tests.yml index 2932639343..fce125a81d 100644 --- a/.github/workflows/tools-tests.yml +++ b/.github/workflows/tools-tests.yml @@ -8,6 +8,23 @@ on: paths: - "tools/**" - ".agents/**" + - ".github/scripts/**" + # The three .github entries below are not cosmetic. Several tests load + # the REAL repo config rather than a fixture, so editing one of these + # files can break the suite even though nothing under tools/ changed: + # .github/policy.yml + # tools/tests/test_check_frozen_paths.py:: + # test_real_policy_file_declares_the_retired_roots calls + # load_frozen_paths() with no argument, which falls back to + # POLICY_PATH (tools/check_frozen_paths.py:38). + # .github/schemas/manifest-schema.json + # test_validate_manifest.py and test_validate_structure.py call + # load_schema(), which hardcodes SCHEMA_PATH and takes no override + # parameter (tools/validate_manifest.py:40,113). + # Without these entries such a PR merges green and the failure only + # surfaces later, on an unrelated PR that happens to touch tools/. + - ".github/policy.yml" + - ".github/schemas/**" - "pyproject.toml" - "uv.lock" - ".github/workflows/tools-tests.yml" @@ -15,8 +32,14 @@ on: branches: - main paths: + # Keep in sync with the pull_request list above (GitHub Actions does + # not support YAML anchors, so the duplication is unavoidable). See + # that block for why the .github/ entries are required. - "tools/**" - ".agents/**" + - ".github/scripts/**" + - ".github/policy.yml" + - ".github/schemas/**" - "pyproject.toml" - "uv.lock" - ".github/workflows/tools-tests.yml" @@ -25,6 +48,12 @@ on: permissions: contents: read +# Cancel superseded runs on the same PR (or ref for push/dispatch) so a rapid +# series of pushes doesn't queue up redundant runs of the same suite. +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + jobs: pytest: name: pytest (tools + skills) @@ -36,16 +65,20 @@ jobs: steps: - name: Checkout repository - uses: actions/checkout@v4 + uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # ratchet:actions/checkout@v4 - name: Install uv - uses: astral-sh/setup-uv@v6 + uses: astral-sh/setup-uv@445689ea25e0de0a23313031f5fe577c74ae45a1 # ratchet:astral-sh/setup-uv@v6.3.0 with: # Test against the minimum supported Python version. python-version: "3.11" - name: Install dependencies (incl. dev group) - run: uv sync --dev + # --locked keeps this job hermetic: install exactly what uv.lock + # pins, never silently re-resolve. Lockfile-drift ENFORCEMENT is not + # this workflow's job — python-dependency-policy.yml (Check 5, `uv + # lock --check`) owns that and emits a far more actionable message. + run: uv sync --locked --dev - name: Run pytest run: uv run pytest diff --git a/.github/workflows/validate-recipe-structure.yml b/.github/workflows/validate-recipe-structure.yml index 341189dcf8..c94ce25472 100644 --- a/.github/workflows/validate-recipe-structure.yml +++ b/.github/workflows/validate-recipe-structure.yml @@ -6,10 +6,23 @@ name: Validate Recipe Structure on: pull_request: + # `labeled` is included so that applying the `allow-legacy-path` escape + # hatch re-runs the retired-folder check instead of needing a manual + # re-run. The other three are the GitHub defaults, which listing + # `types` at all would otherwise drop. + types: [opened, synchronize, reopened, labeled] paths: - 'core/**' - 'contrib/**' - 'skills/**' + # Retired recipe roots (policy.yml `frozen_paths`). Without these the + # workflow never fires for a PR that only touches a retired folder, + # and the check below could never see it. + - 'python/agents/**' + - 'java/agents/**' + - 'go/agents/**' + - 'kotlin/agents/**' + - 'typescript/agents/**' - '.github/schemas/manifest-schema.json' - '.github/policy.yml' - '.github/workflows/validate-recipe-structure.yml' @@ -19,6 +32,8 @@ on: - 'tools/validate_structure.py' - 'tools/validate_readme.py' - 'tools/affected_recipes.py' + - 'tools/check_frozen_paths.py' + - 'tools/validate_placement.py' - 'pyproject.toml' push: branches: @@ -52,12 +67,12 @@ jobs: steps: - name: Checkout repository - uses: actions/checkout@v4 + uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # ratchet:actions/checkout@v4 with: fetch-depth: 0 - name: Install uv - uses: astral-sh/setup-uv@v6 + uses: astral-sh/setup-uv@445689ea25e0de0a23313031f5fe577c74ae45a1 # ratchet:astral-sh/setup-uv@v6.3.0 with: # Pin explicitly for reproducibility and to match the sibling # python-validate-recipe.yml step. Structure checks only need @@ -166,6 +181,93 @@ jobs: exit 1 fi + check-frozen-paths: + name: Check for work in retired folders + runs-on: ubuntu-latest + timeout-minutes: 5 + + # Pull requests only — on a push to main the merge has already happened, + # so failing there is noise rather than a gate. + # + # Skipped for bot authors: Dependabot still has open PRs touching the + # retired roots, and blocking them would just generate churn. Matching on + # the `[bot]` suffix rather than naming Dependabot covers future bots too. + # + # Skipped when a maintainer applies `allow-legacy-path`. Only users with + # write access can label a PR, so this is the same permission boundary as + # an admin override — but it is explicit and recorded on the PR. + if: >- + github.event_name == 'pull_request' && + !endsWith(github.event.pull_request.user.login, '[bot]') && + !contains(github.event.pull_request.labels.*.name, 'allow-legacy-path') + + steps: + - name: Checkout repository + uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # ratchet:actions/checkout@v4 + with: + # Full history so the merge-base diff below resolves. + fetch-depth: 0 + + - name: Install uv + uses: astral-sh/setup-uv@445689ea25e0de0a23313031f5fe577c74ae45a1 # ratchet:astral-sh/setup-uv@v6.3.0 + with: + python-version: "3.11" + enable-cache: false + + - name: Install validation dependencies + # tools/check_frozen_paths.py reads .github/policy.yml, so it needs + # PyYAML from the repo-root pyproject.toml. + run: uv sync + + - name: Check for changes in retired recipe folders + env: + # Route the GitHub-controlled value through an env var rather than + # interpolating it directly, same as the sibling job above. + BASE_REF: ${{ github.event.pull_request.base.ref }} + run: | + set -euo pipefail + + git fetch origin "$BASE_REF" + + # --diff-filter=AM keeps additions and modifications only. + # Deletions and renames are deliberately excluded so that MOVING a + # recipe out of a retired folder is never blocked by this check. + git diff --diff-filter=AM --name-only "origin/$BASE_REF"...HEAD \ + | uv run python tools/check_frozen_paths.py + + check-recipe-placement: + name: Check recipe placement + runs-on: ubuntu-latest + timeout-minutes: 5 + + # Deliberately unconditional — unlike check-frozen-paths there is no bot + # or label exemption. A whole-tree scan, so it also catches a + # misplacement that arrives by rename or bad merge rather than by an + # obvious "add a recipe" diff. + # + # It cannot ride on the validate-structure job above: that runs the + # validators per AFFECTED recipe, and affected_recipes.py maps a + # misplaced skill (skills/ with no vertical) to None, so it + # would never appear in changed_recipes. + + steps: + - name: Checkout repository + uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # ratchet:actions/checkout@v4 + + - name: Install uv + uses: astral-sh/setup-uv@445689ea25e0de0a23313031f5fe577c74ae45a1 # ratchet:astral-sh/setup-uv@v6.3.0 + with: + python-version: "3.11" + enable-cache: false + + - name: Install validation dependencies + # validate_placement imports validate_manifest, which needs PyYAML + # from the repo-root pyproject.toml. + run: uv sync + + - name: Check every recipe sits at its required path + run: uv run python tools/validate_placement.py + check-dependabot-config: name: Check dependabot.yml is up to date runs-on: ubuntu-latest @@ -173,10 +275,10 @@ jobs: steps: - name: Checkout repository - uses: actions/checkout@v4 + uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # ratchet:actions/checkout@v4 - name: Set up Python - uses: actions/setup-python@v5 + uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # ratchet:actions/setup-python@v5 with: python-version: "3.11" diff --git a/AGENTS.md b/AGENTS.md index 88c705b0be..97aac18746 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -4,9 +4,13 @@ All agents must follow the guidelines below without being reminded. ## General - Use the term **recipe** instead of **sample** everywhere — responses, code comments, commit messages, PR descriptions, docs. +- Two different things are called "skills"; keep the terms straight: + - **Vertical skills** — recipes under `skills///` + (e.g. `skills/retail/store-ops/`). Shipped to users. + - **Repo skills** — AI coding-assistant helpers under `.agents/skills/` + (e.g. `prepare-python-recipe`). Used to build this repo. - Recipes live under `core/` (curated), `contrib/` (community), or - `skills/` (vertical skills). **Tooling skills** (AI coding-assistant - helpers) live under `.agents/skills/` — these are a completely separate + `skills/` (vertical skills). Repo skills are a completely separate concept. Do NOT mix `.agents/skills/` changes and recipe/vertical-skill changes in the same PR. If a tool run modifies files outside your task's scope (e.g. a repo-wide ruff sweep touching an unrelated recipe), revert @@ -36,7 +40,11 @@ All agents must follow the guidelines below without being reminded. - Do NOT use `gemini-2.0-flash` or `gemini-2.5-flash` — both are deprecated. Use `gemini-3.5-flash` instead. ## Python -- Python recipes go under `contrib/python/`, `core/python/`, or `skills/python/` +- Python recipes go under `contrib/python/` or `core/python/`. Vertical + skills go under `skills///` (e.g. + `skills/retail/store-ops/`) — that middle folder is a **vertical**, not a + language, and it is mandatory. A skill's language comes from + `manifest.language`. - Minimum python version: 3.11 - Package manager: Use `uv`, not `pip` - Formatter/linter: `ruff` — line length 80, double quotes. Config lives diff --git a/README.md b/README.md index 49a22ca0fe..351f663926 100644 --- a/README.md +++ b/README.md @@ -47,9 +47,11 @@ context and tooling reference, the [Recipe Handbook](./docs/recipe-handbook/README.md) has the deep detail. -Contributor workflow skills (recipe scaffolding, manifest -generation, pyproject alignment, and more) live in -[`.agents/skills/`](./.agents/skills/). +**Repo skills** — the AI coding-assistant helpers used to build this +repo (recipe scaffolding, manifest generation, pyproject alignment, +and more) — live in [`.agents/skills/`](./.agents/skills/). Not to be +confused with **vertical skills**, which are recipes shipped to users +under `skills///`. ## Getting help diff --git a/core/python/ambient-expense-agent/frontend/uv.lock b/core/python/ambient-expense-agent/frontend/uv.lock index 7f7d00c427..2c4f6490d6 100644 --- a/core/python/ambient-expense-agent/frontend/uv.lock +++ b/core/python/ambient-expense-agent/frontend/uv.lock @@ -5,7 +5,7 @@ requires-python = ">=3.11, <3.13" [[package]] name = "annotated-doc" version = "0.0.4" -source = { registry = "https://pypi.org/simple" } +source = { registry = "https://pypi.org/simple/" } sdist = { url = "https://files.pythonhosted.org/packages/57/ba/046ceea27344560984e26a590f90bc7f4a75b06701f653222458922b558c/annotated_doc-0.0.4.tar.gz", hash = "sha256:fbcda96e87e9c92ad167c2e53839e57503ecfda18804ea28102353485033faa4", size = 7288, upload-time = "2025-11-10T22:07:42.062Z" } wheels = [ { url = "https://files.pythonhosted.org/packages/1e/d3/26bf1008eb3d2daa8ef4cacc7f3bfdc11818d111f7e2d0201bc6e3b49d45/annotated_doc-0.0.4-py3-none-any.whl", hash = "sha256:571ac1dc6991c450b25a9c2d84a3705e2ae7a53467b5d111c24fa8baabbed320", size = 5303, upload-time = "2025-11-10T22:07:40.673Z" }, @@ -14,7 +14,7 @@ wheels = [ [[package]] name = "annotated-types" version = "0.7.0" -source = { registry = "https://pypi.org/simple" } +source = { registry = "https://pypi.org/simple/" } sdist = { url = "https://files.pythonhosted.org/packages/ee/67/531ea369ba64dcff5ec9c3402f9f51bf748cec26dde048a2f973a4eea7f5/annotated_types-0.7.0.tar.gz", hash = "sha256:aff07c09a53a08bc8cfccb9c85b05f1aa9a2a6f23728d790723543408344ce89", size = 16081, upload-time = "2024-05-20T21:33:25.928Z" } wheels = [ { url = "https://files.pythonhosted.org/packages/78/b6/6307fbef88d9b5ee7421e68d78a9f162e0da4900bc5f5793f6d3d0e34fb8/annotated_types-0.7.0-py3-none-any.whl", hash = "sha256:1f02e8b43a8fbbc3f3e0d4f0f4bfc8131bcb4eebe8849b8e5c773f3a1c582a53", size = 13643, upload-time = "2024-05-20T21:33:24.1Z" }, @@ -23,7 +23,7 @@ wheels = [ [[package]] name = "anyio" version = "4.13.0" -source = { registry = "https://pypi.org/simple" } +source = { registry = "https://pypi.org/simple/" } dependencies = [ { name = "idna" }, { name = "typing-extensions" }, @@ -36,7 +36,7 @@ wheels = [ [[package]] name = "certifi" version = "2026.2.25" -source = { registry = "https://pypi.org/simple" } +source = { registry = "https://pypi.org/simple/" } sdist = { url = "https://files.pythonhosted.org/packages/af/2d/7bf41579a8986e348fa033a31cdd0e4121114f6bce2457e8876010b092dd/certifi-2026.2.25.tar.gz", hash = "sha256:e887ab5cee78ea814d3472169153c2d12cd43b14bd03329a39a9c6e2e80bfba7", size = 155029, upload-time = "2026-02-25T02:54:17.342Z" } wheels = [ { url = "https://files.pythonhosted.org/packages/9a/3c/c17fb3ca2d9c3acff52e30b309f538586f9f5b9c9cf454f3845fc9af4881/certifi-2026.2.25-py3-none-any.whl", hash = "sha256:027692e4402ad994f1c42e52a4997a9763c646b73e4096e4d5d6db8af1d6f0fa", size = 153684, upload-time = "2026-02-25T02:54:15.766Z" }, @@ -45,7 +45,7 @@ wheels = [ [[package]] name = "cffi" version = "2.0.0" -source = { registry = "https://pypi.org/simple" } +source = { registry = "https://pypi.org/simple/" } dependencies = [ { name = "pycparser", marker = "implementation_name != 'PyPy'" }, ] @@ -81,7 +81,7 @@ wheels = [ [[package]] name = "charset-normalizer" version = "3.4.7" -source = { registry = "https://pypi.org/simple" } +source = { registry = "https://pypi.org/simple/" } sdist = { url = "https://files.pythonhosted.org/packages/e7/a1/67fe25fac3c7642725500a3f6cfe5821ad557c3abb11c9d20d12c7008d3e/charset_normalizer-3.4.7.tar.gz", hash = "sha256:ae89db9e5f98a11a4bf50407d4363e7b09b31e55bc117b4f7d80aab97ba009e5", size = 144271, upload-time = "2026-04-02T09:28:39.342Z" } wheels = [ { url = "https://files.pythonhosted.org/packages/c2/d7/b5b7020a0565c2e9fa8c09f4b5fa6232feb326b8c20081ccded47ea368fd/charset_normalizer-3.4.7-cp311-cp311-macosx_10_9_universal2.whl", hash = "sha256:7641bb8895e77f921102f72833904dcd9901df5d6d72a2ab8f31d04b7e51e4e7", size = 309705, upload-time = "2026-04-02T09:26:02.191Z" }, @@ -122,7 +122,7 @@ wheels = [ [[package]] name = "click" version = "8.3.2" -source = { registry = "https://pypi.org/simple" } +source = { registry = "https://pypi.org/simple/" } dependencies = [ { name = "colorama", marker = "sys_platform == 'win32'" }, ] @@ -134,7 +134,7 @@ wheels = [ [[package]] name = "colorama" version = "0.4.6" -source = { registry = "https://pypi.org/simple" } +source = { registry = "https://pypi.org/simple/" } sdist = { url = "https://files.pythonhosted.org/packages/d8/53/6f443c9a4a8358a93a6792e2acffb9d9d5cb0a5cfd8802644b7b1c9a02e4/colorama-0.4.6.tar.gz", hash = "sha256:08695f5cb7ed6e0531a20572697297273c47b8cae5a63ffc6d6ed5c201be6e44", size = 27697, upload-time = "2022-10-25T02:36:22.414Z" } wheels = [ { url = "https://files.pythonhosted.org/packages/d1/d6/3965ed04c63042e047cb6a3e6ed1a63a35087b6a609aa3a15ed8ac56c221/colorama-0.4.6-py2.py3-none-any.whl", hash = "sha256:4f1d9991f5acc0ca119f9d443620b77f9d6b33703e51011c16baf57afb285fc6", size = 25335, upload-time = "2022-10-25T02:36:20.889Z" }, @@ -143,7 +143,7 @@ wheels = [ [[package]] name = "cryptography" version = "46.0.7" -source = { registry = "https://pypi.org/simple" } +source = { registry = "https://pypi.org/simple/" } dependencies = [ { name = "cffi", marker = "platform_python_implementation != 'PyPy'" }, ] @@ -209,7 +209,7 @@ requires-dist = [ [[package]] name = "fastapi" version = "0.135.3" -source = { registry = "https://pypi.org/simple" } +source = { registry = "https://pypi.org/simple/" } dependencies = [ { name = "annotated-doc" }, { name = "pydantic" }, @@ -225,7 +225,7 @@ wheels = [ [[package]] name = "google-auth" version = "2.49.2" -source = { registry = "https://pypi.org/simple" } +source = { registry = "https://pypi.org/simple/" } dependencies = [ { name = "cryptography" }, { name = "pyasn1-modules" }, @@ -238,7 +238,7 @@ wheels = [ [[package]] name = "h11" version = "0.16.0" -source = { registry = "https://pypi.org/simple" } +source = { registry = "https://pypi.org/simple/" } sdist = { url = "https://files.pythonhosted.org/packages/01/ee/02a2c011bdab74c6fb3c75474d40b3052059d95df7e73351460c8588d963/h11-0.16.0.tar.gz", hash = "sha256:4e35b956cf45792e4caa5885e69fba00bdbc6ffafbfa020300e549b208ee5ff1", size = 101250, upload-time = "2025-04-24T03:35:25.427Z" } wheels = [ { url = "https://files.pythonhosted.org/packages/04/4b/29cac41a4d98d144bf5f6d33995617b185d14b22401f75ca86f384e87ff1/h11-0.16.0-py3-none-any.whl", hash = "sha256:63cf8bbe7522de3bf65932fda1d9c2772064ffb3dae62d55932da54b31cb6c86", size = 37515, upload-time = "2025-04-24T03:35:24.344Z" }, @@ -247,7 +247,7 @@ wheels = [ [[package]] name = "httpcore" version = "1.0.9" -source = { registry = "https://pypi.org/simple" } +source = { registry = "https://pypi.org/simple/" } dependencies = [ { name = "certifi" }, { name = "h11" }, @@ -260,7 +260,7 @@ wheels = [ [[package]] name = "httpx" version = "0.28.1" -source = { registry = "https://pypi.org/simple" } +source = { registry = "https://pypi.org/simple/" } dependencies = [ { name = "anyio" }, { name = "certifi" }, @@ -275,7 +275,7 @@ wheels = [ [[package]] name = "idna" version = "3.11" -source = { registry = "https://pypi.org/simple" } +source = { registry = "https://pypi.org/simple/" } sdist = { url = "https://files.pythonhosted.org/packages/6f/6d/0703ccc57f3a7233505399edb88de3cbd678da106337b9fcde432b65ed60/idna-3.11.tar.gz", hash = "sha256:795dafcc9c04ed0c1fb032c2aa73654d8e8c5023a7df64a53f39190ada629902", size = 194582, upload-time = "2025-10-12T14:55:20.501Z" } wheels = [ { url = "https://files.pythonhosted.org/packages/0e/61/66938bbb5fc52dbdf84594873d5b51fb1f7c7794e9c0f5bd885f30bc507b/idna-3.11-py3-none-any.whl", hash = "sha256:771a87f49d9defaf64091e6e6fe9c18d4833f140bd19464795bc32d966ca37ea", size = 71008, upload-time = "2025-10-12T14:55:18.883Z" }, @@ -284,7 +284,7 @@ wheels = [ [[package]] name = "pyasn1" version = "0.6.3" -source = { registry = "https://pypi.org/simple" } +source = { registry = "https://pypi.org/simple/" } sdist = { url = "https://files.pythonhosted.org/packages/5c/5f/6583902b6f79b399c9c40674ac384fd9cd77805f9e6205075f828ef11fb2/pyasn1-0.6.3.tar.gz", hash = "sha256:697a8ecd6d98891189184ca1fa05d1bb00e2f84b5977c481452050549c8a72cf", size = 148685, upload-time = "2026-03-17T01:06:53.382Z" } wheels = [ { url = "https://files.pythonhosted.org/packages/5d/a0/7d793dce3fa811fe047d6ae2431c672364b462850c6235ae306c0efd025f/pyasn1-0.6.3-py3-none-any.whl", hash = "sha256:a80184d120f0864a52a073acc6fc642847d0be408e7c7252f31390c0f4eadcde", size = 83997, upload-time = "2026-03-17T01:06:52.036Z" }, @@ -293,7 +293,7 @@ wheels = [ [[package]] name = "pyasn1-modules" version = "0.4.2" -source = { registry = "https://pypi.org/simple" } +source = { registry = "https://pypi.org/simple/" } dependencies = [ { name = "pyasn1" }, ] @@ -305,7 +305,7 @@ wheels = [ [[package]] name = "pycparser" version = "3.0" -source = { registry = "https://pypi.org/simple" } +source = { registry = "https://pypi.org/simple/" } sdist = { url = "https://files.pythonhosted.org/packages/1b/7d/92392ff7815c21062bea51aa7b87d45576f649f16458d78b7cf94b9ab2e6/pycparser-3.0.tar.gz", hash = "sha256:600f49d217304a5902ac3c37e1281c9fe94e4d0489de643a9504c5cdfdfc6b29", size = 103492, upload-time = "2026-01-21T14:26:51.89Z" } wheels = [ { url = "https://files.pythonhosted.org/packages/0c/c3/44f3fbbfa403ea2a7c779186dc20772604442dde72947e7d01069cbe98e3/pycparser-3.0-py3-none-any.whl", hash = "sha256:b727414169a36b7d524c1c3e31839a521725078d7b2ff038656844266160a992", size = 48172, upload-time = "2026-01-21T14:26:50.693Z" }, @@ -314,7 +314,7 @@ wheels = [ [[package]] name = "pydantic" version = "2.13.0" -source = { registry = "https://pypi.org/simple" } +source = { registry = "https://pypi.org/simple/" } dependencies = [ { name = "annotated-types" }, { name = "pydantic-core" }, @@ -329,7 +329,7 @@ wheels = [ [[package]] name = "pydantic-core" version = "2.46.0" -source = { registry = "https://pypi.org/simple" } +source = { registry = "https://pypi.org/simple/" } dependencies = [ { name = "typing-extensions" }, ] @@ -386,7 +386,7 @@ wheels = [ [[package]] name = "requests" version = "2.33.1" -source = { registry = "https://pypi.org/simple" } +source = { registry = "https://pypi.org/simple/" } dependencies = [ { name = "certifi" }, { name = "charset-normalizer" }, @@ -401,7 +401,7 @@ wheels = [ [[package]] name = "starlette" version = "1.0.0" -source = { registry = "https://pypi.org/simple" } +source = { registry = "https://pypi.org/simple/" } dependencies = [ { name = "anyio" }, { name = "typing-extensions" }, @@ -414,7 +414,7 @@ wheels = [ [[package]] name = "typing-extensions" version = "4.15.0" -source = { registry = "https://pypi.org/simple" } +source = { registry = "https://pypi.org/simple/" } sdist = { url = "https://files.pythonhosted.org/packages/72/94/1a15dd82efb362ac84269196e94cf00f187f7ed21c242792a923cdb1c61f/typing_extensions-4.15.0.tar.gz", hash = "sha256:0cea48d173cc12fa28ecabc3b837ea3cf6f38c6d1136f85cbaaf598984861466", size = 109391, upload-time = "2025-08-25T13:49:26.313Z" } wheels = [ { url = "https://files.pythonhosted.org/packages/18/67/36e9267722cc04a6b9f15c7f3441c2363321a3ea07da7ae0c0707beb2a9c/typing_extensions-4.15.0-py3-none-any.whl", hash = "sha256:f0fa19c6845758ab08074a0cfa8b7aecb71c999ca73d62883bc25cc018c4e548", size = 44614, upload-time = "2025-08-25T13:49:24.86Z" }, @@ -423,7 +423,7 @@ wheels = [ [[package]] name = "typing-inspection" version = "0.4.2" -source = { registry = "https://pypi.org/simple" } +source = { registry = "https://pypi.org/simple/" } dependencies = [ { name = "typing-extensions" }, ] @@ -435,7 +435,7 @@ wheels = [ [[package]] name = "urllib3" version = "2.6.3" -source = { registry = "https://pypi.org/simple" } +source = { registry = "https://pypi.org/simple/" } sdist = { url = "https://files.pythonhosted.org/packages/c7/24/5f1b3bdffd70275f6661c76461e25f024d5a38a46f04aaca912426a2b1d3/urllib3-2.6.3.tar.gz", hash = "sha256:1b62b6884944a57dbe321509ab94fd4d3b307075e0c2eae991ac71ee15ad38ed", size = 435556, upload-time = "2026-01-07T16:24:43.925Z" } wheels = [ { url = "https://files.pythonhosted.org/packages/39/08/aaaad47bc4e9dc8c725e68f9d04865dbcb2052843ff09c97b08904852d84/urllib3-2.6.3-py3-none-any.whl", hash = "sha256:bf272323e553dfb2e87d9bfd225ca7b0f467b919d7bbd355436d3fd37cb0acd4", size = 131584, upload-time = "2026-01-07T16:24:42.685Z" }, @@ -444,7 +444,7 @@ wheels = [ [[package]] name = "uvicorn" version = "0.44.0" -source = { registry = "https://pypi.org/simple" } +source = { registry = "https://pypi.org/simple/" } dependencies = [ { name = "click" }, { name = "h11" }, diff --git a/docs/recipe-checklist.md b/docs/recipe-checklist.md index bd4fbf6367..cb893016b7 100644 --- a/docs/recipe-checklist.md +++ b/docs/recipe-checklist.md @@ -1,4 +1,4 @@ - + # Recipe Checklist @@ -24,7 +24,7 @@ skill in the right order. | `generate-python-runnability-test` | Writes `tests/test_runnability.py` | `generate runnability test for contrib/python/my-recipe` | For deep detail on each skill, see the -[Skills Catalog](./recipe-handbook/skills-catalog.md). +[Repo Skills Catalog](./recipe-handbook/skills-catalog.md). --- diff --git a/docs/recipe-handbook/README.md b/docs/recipe-handbook/README.md index 547f6f96c2..1feb6e6762 100644 --- a/docs/recipe-handbook/README.md +++ b/docs/recipe-handbook/README.md @@ -1,4 +1,4 @@ - + # Recipe Handbook @@ -35,7 +35,8 @@ it covers everything on one page. Come back here for deeper context: recipes, regardless of language - [Python language rules](./languages/python.md) — starts with the fast path; specific requirements and end-to-end scenarios -- [Skills catalog](./skills-catalog.md) — AI skills reference +- [Repo skills catalog](./skills-catalog.md) — the assistant + helpers that build this repo **Updating an existing recipe?** Run `prepare-python-recipe` against your recipe path — it's safe to re-run and applies @@ -44,8 +45,8 @@ any new requirements automatically. Then check the **Reference:** -- [Skills catalog](./skills-catalog.md) — the AI skills that - do the work for you +- [Repo skills catalog](./skills-catalog.md) — the assistant + helpers that do the work for you - [Troubleshooting](./troubleshooting.md) — errors mapped directly to fixes - Other languages *(coming soon)*: Java · Go · TypeScript · @@ -56,10 +57,16 @@ any new requirements automatically. Then check the - **Recipe** — a runnable agent example (or importable agent module) under `contrib/`, consumed by ADK developers and coding agents alike. -- **Skill** — a pre-loaded instruction set that your AI coding +- **Repo skill** — a pre-loaded instruction set that your AI coding assistant follows when you ask it to perform a task (e.g. - `prepare-python-recipe`). Skill files live in `.agents/skills/` - and load automatically when you open this repo. + `prepare-python-recipe`). Files live in `.agents/skills/` and load + automatically when you open this repo. Repo skills *build* the + repo; they are never shipped to users. +- **Vertical skill** — a recipe under + `skills///` (e.g. `skills/retail/store-ops/`), + where the vertical names the business domain that owns it. + Shipped to users like any other recipe. Unrelated to repo skills, + despite the shared word. - **Manifest** — `manifest.yaml`. Declares recipe metadata: type, language, ownership, description. - **Runnability test** — a smoke test that imports the agent module diff --git a/docs/recipe-handbook/skills-catalog.md b/docs/recipe-handbook/skills-catalog.md index f87004989e..5e342a10c1 100644 --- a/docs/recipe-handbook/skills-catalog.md +++ b/docs/recipe-handbook/skills-catalog.md @@ -1,12 +1,17 @@ - + -# AI Skills Catalog +# Repo Skills Catalog -Skills your AI coding assistant invokes to help you prepare a recipe. Source lives at +**Repo skills** your AI coding assistant invokes to help you prepare a +recipe. Source lives at [`.agents/skills/`](../../.agents/skills/); each has a `SKILL.md` with a full description. This catalog summarises them and maps them to the [checklist](../recipe-checklist.md). +> Not to be confused with **vertical skills** — recipes shipped to +> users under `skills///`. Repo skills build this +> repo; vertical skills are built with it. + > **Fastest path:** for a PR-ready recipe in one command, use > [`prepare-python-recipe`](#prepare-python-recipe). It runs > every other Python skill in the right order. diff --git a/go/README.md b/go/README.md index a93689ee69..87f3e4e030 100644 --- a/go/README.md +++ b/go/README.md @@ -1,5 +1,18 @@ # Agent Development Kit (ADK) Go Samples +> [!IMPORTANT] +> **This folder is retired.** `go/agents/` no longer accepts new +> recipes, or changes to the recipes already here. Recipes now live in +> **`contrib/go/`**. +> +> - **Contributing a new recipe?** Start with the +> [recipe checklist](../docs/recipe-checklist.md). +> - **Already have a recipe here?** Move it to `contrib/go/` and +> follow the same checklist. See the +> [contributor guide](../docs/README.md). +> +> Pull requests that add or modify files under `go/agents/` fail CI. + [![License](https://img.shields.io/badge/License-Apache_2.0-blue.svg)](LICENSE) Agent Development Kit Logo diff --git a/java/README.md b/java/README.md index dcd9ca44e9..fcfc516e38 100644 --- a/java/README.md +++ b/java/README.md @@ -1,5 +1,17 @@ # Sample Agents (Java) +> [!IMPORTANT] +> **This folder is retired.** `java/agents/` no longer accepts new +> recipes, or changes to the recipes already here. Recipes now live in +> **`contrib/java/`**. +> +> - **Contributing a new recipe?** Start with the +> [recipe checklist](../docs/recipe-checklist.md). +> - **Already have a recipe here?** Move it to `contrib/java/` and +> follow the same checklist. See the +> [contributor guide](../docs/README.md). +> +> Pull requests that add or modify files under `java/agents/` fail CI. This folder contains Java agent samples for the [Agent Development Kit](https://github.com/google/adk-java) (ADK). diff --git a/kotlin/README.md b/kotlin/README.md index 0963cd9327..4c0386834b 100644 --- a/kotlin/README.md +++ b/kotlin/README.md @@ -1,5 +1,18 @@ # Agent Development Kit (ADK) Kotlin Samples +> [!IMPORTANT] +> **This folder is retired.** `kotlin/agents/` no longer accepts new +> recipes, or changes to the recipes already here. Recipes now live in +> **`contrib/kotlin/`**. +> +> - **Contributing a new recipe?** Start with the +> [recipe checklist](../docs/recipe-checklist.md). +> - **Already have a recipe here?** Move it to `contrib/kotlin/` and +> follow the same checklist. See the +> [contributor guide](../docs/README.md). +> +> Pull requests that add or modify files under `kotlin/agents/` fail CI. + [![License](https://img.shields.io/badge/License-Apache_2.0-blue.svg)](LICENSE) Agent Development Kit Logo diff --git a/pyproject.toml b/pyproject.toml index 62c965808f..649ebf25b1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -94,9 +94,14 @@ indent-style = "space" addopts = "--import-mode=importlib" # Only the repo tooling and skill scripts are tested here. Recipe tests under # core/ and contrib/ have their own per-recipe suites (see python-tests.yml). +# .github/scripts has no tests yet; it is listed so that any added later are +# collected without a config change. Those scripts gate real PRs (e.g. +# check_env_vars.py and check_recipe_pyproject.py run in +# python-validate-recipe.yml) and are currently uncovered. testpaths = [ "tools/tests", ".agents/skills", + ".github/scripts", ] # Never descend into skill template payloads or virtualenvs. The scaffold # skill ships template test files under resources/templates/tests/ that contain diff --git a/python/README.md b/python/README.md index 2bd3d54cbc..7a2c9b6d46 100644 --- a/python/README.md +++ b/python/README.md @@ -1,5 +1,18 @@ # Agent Development Kit (ADK) Python Samples +> [!IMPORTANT] +> **This folder is retired.** `python/agents/` no longer accepts new +> recipes, or changes to the recipes already here. Recipes now live in +> **`contrib/python/`**. +> +> - **Contributing a new recipe?** Start with the +> [recipe checklist](../docs/recipe-checklist.md). +> - **Already have a recipe here?** Move it to `contrib/python/` and +> follow the same checklist. See the +> [contributor guide](../docs/README.md). +> +> Pull requests that add or modify files under `python/agents/` fail CI. + [![License](https://img.shields.io/badge/License-Apache_2.0-blue.svg)](LICENSE) Agent Development Kit Logo diff --git a/skills/python/.gitkeep b/skills/.gitkeep similarity index 100% rename from skills/python/.gitkeep rename to skills/.gitkeep diff --git a/tools/affected_recipes.py b/tools/affected_recipes.py index 0d3541d0e6..f6707e6192 100644 --- a/tools/affected_recipes.py +++ b/tools/affected_recipes.py @@ -8,12 +8,15 @@ the set of recipes that need re-validation. Layout rules (kept in sync with tools/validate_manifest.py): - //… flat - ///… language-namespaced + //… flat (core / contrib) + ///… language-nsed (core / contrib) + skills///… vertical-nsed (skills) where: ∈ RECIPE_ROOTS (core / contrib / skills) ∈ LANGUAGE_NAMESPACE_DIRS (python / java / go / …) + is free-form (retail / hr / finance / …) and mandatory — + see NAMESPACE_REQUIRED_ROOTS in validate_manifest.py A path whose component doesn't correspond to a real directory on disk is dropped by default — this filters out top-level files like @@ -77,6 +80,18 @@ def recipe_dir_for(path: str) -> str | None: if not part1: return None + # Roots where the namespace is mandatory (skills//). + # The vertical is free-form, so it can only be recognised by position: + # a path identifies a solution only when there is something BELOW it, + # i.e. at least root/vertical/solution/. Anything shallower — + # `skills/foo/SKILL.md`, a solution placed directly under the root — + # deliberately maps to None rather than inventing a recipe at the wrong + # depth. tools/validate_placement.py reports those as misplaced. + if root in vm.NAMESPACE_REQUIRED_ROOTS: + if len(parts) >= 4 and part2: + return f"{root}/{part1}/{part2}" + return None + # Language-namespaced: root/language/recipe/… if part1 in LANGUAGE_NAMESPACE_DIRS and part2: return f"{root}/{part1}/{part2}" diff --git a/tools/check_frozen_paths.py b/tools/check_frozen_paths.py new file mode 100644 index 0000000000..4c2d283125 --- /dev/null +++ b/tools/check_frozen_paths.py @@ -0,0 +1,173 @@ +#!/usr/bin/env python3 +""" +Reject changes made inside retired recipe folders. + +Recipes used to live under `/agents/` at the repo root +(e.g. `python/agents/academic-research`). Those roots are closed: new +recipes and changes to existing ones belong in +`contrib//`. The retired roots are listed under +`frozen_paths` in .github/policy.yml. + +Reads changed file paths from stdin (one per line) — the same interface as +tools/affected_recipes.py — and exits non-zero if any of them sit inside a +frozen path. Offending files are grouped by the recipe directory they +belong to, so a large PR produces one annotation per recipe rather than +one per file. + +Callers should pass only ADDED and MODIFIED paths (`git diff +--diff-filter=AM`). Deletions and renames must be excluded so that +migrating a recipe out of a retired folder is not itself blocked. + +Usage: + git diff --diff-filter=AM --name-only origin/main...HEAD | \\ + uv run python tools/check_frozen_paths.py + +Exit codes: + 0 no changes inside a retired folder + 1 at least one change inside a retired folder +""" + +from __future__ import annotations + +import sys +from pathlib import Path + +import yaml + +REPO_ROOT = Path(__file__).parent.parent +POLICY_PATH = REPO_ROOT / ".github" / "policy.yml" + +# Where recipes live now. Only used to build the "move it here" hint. +ACTIVE_CONTRIB_ROOT = "contrib" + +# How many offending recipes to spell out before truncating the summary. +# Every one still gets its own ::error annotation; this only bounds the +# human-readable block at the end. +MAX_LISTED = 20 + + +def load_frozen_paths(policy_path: Path = POLICY_PATH) -> list[str]: + """Return the `frozen_paths` list from policy.yml, normalised. + + A missing or empty section yields an empty list, which makes this + check a no-op rather than a hard failure. + """ + with open(policy_path, encoding="utf-8") as f: + policy = yaml.safe_load(f) or {} + raw = policy.get("frozen_paths") or [] + return [p for p in (str(x).strip().strip("/") for x in raw) if p] + + +def frozen_prefix_for(path: str, frozen_paths: list[str]) -> str | None: + """Return the frozen prefix `path` sits under, or None. + + Matching is on whole path components, so a frozen prefix of + `python/agents` matches `python/agents/foo/bar.py` but never + `python/agents-archive/foo.py`. + """ + parts = path.strip().strip("/").split("/") + for prefix in frozen_paths: + prefix_parts = prefix.split("/") + if parts[: len(prefix_parts)] == prefix_parts: + return prefix + return None + + +def retired_recipe_dir(path: str, prefix: str) -> str: + """The recipe directory `path` belongs to inside a retired folder. + + `python/agents/foo/app/agent.py` with prefix `python/agents` gives + `python/agents/foo`. A path with nothing after the prefix degrades to + the prefix itself. + """ + parts = path.strip().strip("/").split("/") + depth = len(prefix.split("/")) + if len(parts) > depth: + return "/".join(parts[: depth + 1]) + return prefix + + +def suggested_destination(recipe_dir: str) -> str: + """Where a retired recipe should be moved to. + + `python/agents/foo` -> `contrib/python/foo`. Falls back to a generic + hint when the path is too short to name a recipe. + """ + parts = recipe_dir.split("/") + language = parts[0] + if len(parts) >= 3: + return f"{ACTIVE_CONTRIB_ROOT}/{language}/{parts[-1]}" + return f"{ACTIVE_CONTRIB_ROOT}/{language}/" + + +def find_violations( + changed_files: list[str], frozen_paths: list[str] +) -> dict[str, str]: + """Map each offending recipe directory to one example file in it. + + Insertion order follows the first time each recipe directory is seen, + so output is stable for a stable input ordering. + """ + violations: dict[str, str] = {} + for line in changed_files: + path = line.strip() + if not path: + continue + prefix = frozen_prefix_for(path, frozen_paths) + if prefix is None: + continue + violations.setdefault(retired_recipe_dir(path, prefix), path) + return violations + + +def _report(violations: dict[str, str]) -> None: + """Print GitHub annotations plus a human-readable summary.""" + for recipe_dir, example in violations.items(): + destination = suggested_destination(recipe_dir) + print( + f"::error file={example}::" + f"'{recipe_dir}' is in a retired folder and no longer accepts " + f"changes. Move this recipe to '{destination}'. " + f"See docs/recipe-checklist.md." + ) + + count = len(violations) + noun = "recipe" if count == 1 else "recipes" + print("") + print("=" * 60) + print(f" ACTION REQUIRED: {count} {noun} in a retired folder") + print("=" * 60) + print("") + print("These folders are closed. Recipes now live under contrib/.") + print("") + for recipe_dir in list(violations)[:MAX_LISTED]: + print(f" {recipe_dir} -> {suggested_destination(recipe_dir)}") + if count > MAX_LISTED: + print(f" ... and {count - MAX_LISTED} more") + print("") + print("To fix: move the recipe to the path shown above, then make sure") + print("it meets the contribution requirements:") + print("") + print(" docs/recipe-checklist.md") + print("") + + +def main(argv: list[str] | None = None) -> int: + frozen_paths = load_frozen_paths() + if not frozen_paths: + print("[SKIP] No frozen_paths configured in .github/policy.yml.") + return 0 + + changed_files = sys.stdin.read().splitlines() + violations = find_violations(changed_files, frozen_paths) + + if not violations: + print("[PASS] No changes inside retired recipe folders.") + return 0 + + _report(violations) + return 1 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tools/tests/test_affected_recipes.py b/tools/tests/test_affected_recipes.py index ba40507530..2fa1ab310b 100644 --- a/tools/tests/test_affected_recipes.py +++ b/tools/tests/test_affected_recipes.py @@ -31,14 +31,20 @@ ("contrib/python/bar/pyproject.toml", "contrib/python/bar"), ("core/java/hello/pom.xml", "core/java/hello"), ("contrib/go/example/main.go", "contrib/go/example"), - # Skills root (already listed in RECIPE_ROOTS even though the - # directory may not exist yet). - ("skills/foo/SKILL.md", "skills/foo"), - # A language-namespaced recipe under skills/ (e.g. skills/python/foo) - # is recognised too — matches the layout affected_recipes.py accepts - # for core/ and contrib/, and the python-validate-recipe.yml - # full-scan step (which enumerates core, contrib, and skills). - ("skills/python/foo/agent.py", "skills/python/foo"), + # Skills use a MANDATORY vertical namespace, + # skills//. The vertical is free-form, so the + # solution is identified by position rather than by name. + ("skills/retail/store-ops/SKILL.md", "skills/retail/store-ops"), + ("skills/hr/onboarding/scripts/run.py", "skills/hr/onboarding"), + ("skills/finance/close/eval/cases.jsonl", "skills/finance/close"), + # A solution placed directly under skills/, with no vertical, must + # NOT be promoted to a recipe — mapping it would validate it at the + # wrong depth and hide the misplacement. validate_placement.py is + # what reports it. + ("skills/foo/SKILL.md", None), + ("skills/foo/manifest.yaml", None), + # A vertical-level file belongs to no solution. + ("skills/retail/README.md", None), # Paths that don't sit under a recipe root. ("README.md", None), (".github/workflows/x.yml", None), @@ -156,13 +162,22 @@ def test_compute_skips_blank_and_whitespace_lines(tmp_path): def test_compute_handles_skills_root(tmp_path): - _make_recipe(tmp_path, "skills/example") - changed = ["skills/example/SKILL.md"] + _make_recipe(tmp_path, "skills/retail/store-ops") + changed = ["skills/retail/store-ops/SKILL.md"] assert m.compute_affected_recipes(changed, repo_root=tmp_path) == [ - "skills/example", + "skills/retail/store-ops", ] +def test_compute_ignores_a_skill_with_no_vertical(tmp_path): + """A solution dropped straight under skills/ is not collected here — it + would otherwise be validated at the vertical's depth. Reporting it is + tools/validate_placement.py's job.""" + _make_recipe(tmp_path, "skills/no-vertical") + changed = ["skills/no-vertical/SKILL.md"] + assert m.compute_affected_recipes(changed, repo_root=tmp_path) == [] + + def test_compute_returns_empty_on_no_changes(tmp_path): assert m.compute_affected_recipes([], repo_root=tmp_path) == [] @@ -363,18 +378,31 @@ def test_language_filter_case_insensitive_argument(tmp_path): ) == ["core/flat-py"] -def test_language_filter_skills_python_namespaced(tmp_path): - # A Python recipe under skills/python/ is a namespaced Python recipe - # by path convention — the language filter must include it without - # having to read the manifest. Guards the invariant that - # python-validate-recipe.yml's full-scan step (which now enumerates - # skills/ too) will actually see skills/python/ during - # workflow_dispatch or infra-change runs. - _make_recipe_with_manifest(tmp_path, "skills/python/foo", manifest=None) - changed = ["skills/python/foo/SKILL.md"] +def test_language_filter_skills_uses_the_manifest_not_the_path(tmp_path): + # A skill's namespace is a VERTICAL, not a language, so the path cannot + # answer the language question the way core/python/ can. The + # filter must fall through to manifest.language — which works because + # `language` stays a required manifest field for skills. + _make_recipe_with_manifest( + tmp_path, "skills/retail/store-ops", manifest="language: python\n" + ) + changed = ["skills/retail/store-ops/SKILL.md"] assert m.compute_affected_recipes( changed, language="python", repo_root=tmp_path - ) == ["skills/python/foo"] + ) == ["skills/retail/store-ops"] + + +def test_language_filter_skills_excludes_other_languages(tmp_path): + _make_recipe_with_manifest( + tmp_path, "skills/retail/store-ops", manifest="language: java\n" + ) + changed = ["skills/retail/store-ops/SKILL.md"] + assert ( + m.compute_affected_recipes( + changed, language="python", repo_root=tmp_path + ) + == [] + ) def test_language_filter_mixes_flat_and_namespaced(tmp_path): diff --git a/tools/tests/test_check_frozen_paths.py b/tools/tests/test_check_frozen_paths.py new file mode 100644 index 0000000000..e5fd37c718 --- /dev/null +++ b/tools/tests/test_check_frozen_paths.py @@ -0,0 +1,256 @@ +#!/usr/bin/env python3 +"""Unit tests for tools/check_frozen_paths.py. + +The interesting logic is pure string handling, so most tests are tables of +path → expectation. `load_frozen_paths` and `main` touch the filesystem and +stdin respectively and get their own small set of tests. +""" + +from pathlib import Path + +import check_frozen_paths as m +import pytest + +FROZEN = [ + "python/agents", + "java/agents", + "go/agents", + "kotlin/agents", + "typescript/agents", +] + + +# --------------------------------------------------------------------------- +# frozen_prefix_for — which retired root, if any, does a path sit under +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "path,expected", + [ + # Inside retired roots. + ("python/agents/foo/agent.py", "python/agents"), + ("python/agents/foo/tests/test_x.py", "python/agents"), + ("java/agents/hello/pom.xml", "java/agents"), + ("go/agents/example/main.go", "go/agents"), + ("typescript/agents/thing/package.json", "typescript/agents"), + # The retired root itself, and a file directly inside it. + ("python/agents", "python/agents"), + ("python/agents/README.md", "python/agents"), + # Active locations are untouched. + ("contrib/python/foo/agent.py", None), + ("core/python/foo/agent.py", None), + ("skills/python/foo/SKILL.md", None), + # The language README sits OUTSIDE the frozen prefix on purpose — + # the deprecation notice has to stay editable. + ("python/README.md", None), + ("java/README.md", None), + # Repo infrastructure. + ("tools/validate.py", None), + (".github/workflows/python-tests.yml", None), + ("README.md", None), + # Component-boundary matching: a sibling directory whose name merely + # starts with the frozen prefix must not match. + ("python/agents-archive/foo.py", None), + ("pythonic/agents/foo.py", None), + ], +) +def test_frozen_prefix_for(path, expected): + assert m.frozen_prefix_for(path, FROZEN) == expected + + +def test_frozen_prefix_for_tolerates_leading_slash(): + assert m.frozen_prefix_for("/python/agents/foo/x.py", FROZEN) == ( + "python/agents" + ) + + +def test_frozen_prefix_for_empty_config_matches_nothing(): + assert m.frozen_prefix_for("python/agents/foo/x.py", []) is None + + +# --------------------------------------------------------------------------- +# retired_recipe_dir — collapse a file path to the recipe it belongs to +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "path,prefix,expected", + [ + ("python/agents/foo/agent.py", "python/agents", "python/agents/foo"), + ( + "python/agents/foo/app/sub/deep.py", + "python/agents", + "python/agents/foo", + ), + ("java/agents/hello/pom.xml", "java/agents", "java/agents/hello"), + # Nothing after the prefix — degrade to the prefix itself. + ("python/agents", "python/agents", "python/agents"), + ], +) +def test_retired_recipe_dir(path, prefix, expected): + assert m.retired_recipe_dir(path, prefix) == expected + + +# --------------------------------------------------------------------------- +# suggested_destination — the "move it here" hint +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "recipe_dir,expected", + [ + ("python/agents/foo", "contrib/python/foo"), + ("java/agents/hello", "contrib/java/hello"), + ("go/agents/example", "contrib/go/example"), + ("typescript/agents/thing", "contrib/typescript/thing"), + ("kotlin/agents/demo", "contrib/kotlin/demo"), + # Too short to name a recipe — generic hint. + ("python/agents", "contrib/python/"), + ], +) +def test_suggested_destination(recipe_dir, expected): + assert m.suggested_destination(recipe_dir) == expected + + +# --------------------------------------------------------------------------- +# find_violations — grouping and deduplication +# --------------------------------------------------------------------------- + + +def test_no_violations_for_active_paths(): + changed = [ + "contrib/python/foo/agent.py", + "core/python/bar/README.md", + "tools/validate.py", + "python/README.md", + ] + assert m.find_violations(changed, FROZEN) == {} + + +def test_many_files_in_one_recipe_collapse_to_one_entry(): + changed = [ + "python/agents/foo/agent.py", + "python/agents/foo/README.md", + "python/agents/foo/tests/test_agent.py", + ] + violations = m.find_violations(changed, FROZEN) + assert list(violations) == ["python/agents/foo"] + # The example file kept is the first one seen. + assert violations["python/agents/foo"] == "python/agents/foo/agent.py" + + +def test_multiple_recipes_and_languages_each_reported(): + changed = [ + "python/agents/foo/agent.py", + "python/agents/bar/agent.py", + "java/agents/hello/pom.xml", + "contrib/python/ok/agent.py", + ] + violations = m.find_violations(changed, FROZEN) + assert set(violations) == { + "python/agents/foo", + "python/agents/bar", + "java/agents/hello", + } + + +def test_blank_lines_are_ignored(): + changed = ["", " ", "python/agents/foo/agent.py", ""] + assert list(m.find_violations(changed, FROZEN)) == ["python/agents/foo"] + + +def test_mixed_pr_still_flags_the_retired_part(): + """A PR that moves a recipe adds files under contrib/ and touches the + retired copy. Only the retired side is flagged.""" + changed = [ + "contrib/python/foo/agent.py", + "contrib/python/foo/manifest.yaml", + "python/agents/foo/agent.py", + ] + assert list(m.find_violations(changed, FROZEN)) == ["python/agents/foo"] + + +# --------------------------------------------------------------------------- +# load_frozen_paths — policy.yml reading +# --------------------------------------------------------------------------- + + +def _write_policy(tmp_path: Path, body: str) -> Path: + policy = tmp_path / "policy.yml" + policy.write_text(body, encoding="utf-8") + return policy + + +def test_load_frozen_paths_reads_and_normalises(tmp_path): + policy = _write_policy( + tmp_path, + "frozen_paths:\n - python/agents\n - /java/agents/\n", + ) + assert m.load_frozen_paths(policy) == ["python/agents", "java/agents"] + + +@pytest.mark.parametrize( + "body", + [ + "frozen_paths: []\n", + "frozen_paths:\n", + "recipe_naming:\n max_folder_name_length: 30\n", + "", + ], +) +def test_load_frozen_paths_missing_or_empty(tmp_path, body): + assert m.load_frozen_paths(_write_policy(tmp_path, body)) == [] + + +def test_real_policy_file_declares_the_retired_roots(): + """Guards against the policy entry being dropped by accident.""" + frozen = m.load_frozen_paths() + assert "python/agents" in frozen + for language in ("java", "go", "kotlin", "typescript"): + assert f"{language}/agents" in frozen + + +# --------------------------------------------------------------------------- +# main — stdin plumbing and exit codes +# --------------------------------------------------------------------------- + + +def test_main_passes_on_clean_input(monkeypatch, capsys): + monkeypatch.setattr( + "sys.stdin", _FakeStdin("contrib/python/foo/agent.py\n") + ) + assert m.main() == 0 + assert "[PASS]" in capsys.readouterr().out + + +def test_main_fails_and_annotates_on_retired_path(monkeypatch, capsys): + monkeypatch.setattr("sys.stdin", _FakeStdin("python/agents/foo/agent.py\n")) + assert m.main() == 1 + out = capsys.readouterr().out + assert "::error file=python/agents/foo/agent.py::" in out + assert "contrib/python/foo" in out + assert "ACTION REQUIRED" in out + + +def test_main_passes_on_empty_input(monkeypatch, capsys): + monkeypatch.setattr("sys.stdin", _FakeStdin("")) + assert m.main() == 0 + assert "[PASS]" in capsys.readouterr().out + + +def test_main_skips_when_no_frozen_paths(monkeypatch, capsys): + monkeypatch.setattr(m, "load_frozen_paths", lambda *a, **k: []) + monkeypatch.setattr("sys.stdin", _FakeStdin("python/agents/foo/agent.py\n")) + assert m.main() == 0 + assert "[SKIP]" in capsys.readouterr().out + + +class _FakeStdin: + """Minimal stand-in for sys.stdin supporting the single read() call.""" + + def __init__(self, text: str): + self._text = text + + def read(self) -> str: + return self._text diff --git a/tools/tests/test_validate_manifest.py b/tools/tests/test_validate_manifest.py index 41cf49fc51..8e25659843 100644 --- a/tools/tests/test_validate_manifest.py +++ b/tools/tests/test_validate_manifest.py @@ -228,6 +228,92 @@ def test_collect_nonexistent_scope_exits(fake_repo): m.collect_recipe_dirs("core/does-not-exist") +# --------------------------------------------------------------------------- +# skills/ — mandatory vertical namespace (skills//) +# --------------------------------------------------------------------------- + + +@pytest.fixture +def skills_repo(tmp_path, monkeypatch): + """A fake repo laid out as skills//.""" + _make_recipe(tmp_path, "skills/retail/store-ops") + _make_recipe(tmp_path, "skills/hr/onboarding") + _make_recipe(tmp_path, "skills/finance/month-end-close") + monkeypatch.setattr(m, "REPO_ROOT", tmp_path) + return tmp_path + + +def test_collect_skills_returns_solutions_not_verticals(skills_repo): + """The regression this whole layout change exists to prevent: before + NAMESPACE_REQUIRED_ROOTS, this returned the VERTICALS (skills/retail, + skills/hr), so every check ran against the wrong directory and the + real solutions were never validated at all.""" + dirs = m.collect_recipe_dirs("skills") + assert _rel(dirs, skills_repo) == { + "skills/retail/store-ops", + "skills/hr/onboarding", + "skills/finance/month-end-close", + } + + +def test_collect_scoped_to_one_vertical(skills_repo): + dirs = m.collect_recipe_dirs("skills/retail") + assert _rel(dirs, skills_repo) == {"skills/retail/store-ops"} + + +def test_collect_single_solution(skills_repo): + dirs = m.collect_recipe_dirs("skills/retail/store-ops") + assert _rel(dirs, skills_repo) == {"skills/retail/store-ops"} + + +def test_collect_skips_a_solution_with_no_vertical(tmp_path, monkeypatch): + """A solution directly under skills/ is treated as an (empty) vertical + and contributes nothing, rather than being validated at the wrong + depth. validate_placement.py is what reports the misplacement.""" + _make_recipe(tmp_path, "skills/retail/store-ops") + _make_recipe(tmp_path, "skills/no-vertical") + monkeypatch.setattr(m, "REPO_ROOT", tmp_path) + assert _rel(m.collect_recipe_dirs("skills"), tmp_path) == { + "skills/retail/store-ops" + } + + +def test_a_vertical_named_like_a_language_is_still_a_vertical( + tmp_path, monkeypatch +): + """`skills/python/foo` is vertical `python` + solution `foo`, not a + language namespace. The result matches what the old language-based + rule produced, but for a different reason — pinned so a future + refactor cannot quietly reintroduce language semantics under skills/.""" + _make_recipe(tmp_path, "skills/python/foo") + monkeypatch.setattr(m, "REPO_ROOT", tmp_path) + assert _rel(m.collect_recipe_dirs("skills"), tmp_path) == { + "skills/python/foo" + } + + +@pytest.mark.parametrize( + "parts,expected", + [ + # core/contrib: namespace recognised by NAME. + (["core", "python"], True), + (["contrib", "java"], True), + (["core", "my-recipe"], False), + # skills: namespace recognised by POSITION, whatever it is called. + (["skills", "retail"], True), + (["skills", "anything-at-all"], True), + # Depth matters — only the component directly under a root. + (["skills", "retail", "store-ops"], False), + (["core", "python", "foo"], False), + (["core"], False), + # Not a recipe root. + (["python", "agents"], False), + ], +) +def test_is_namespace_path(parts, expected): + assert m.is_namespace_path(parts) is expected + + def test_collect_invalid_recipe_dir_exits(tmp_path, monkeypatch): # A dir with only README.md is not a valid recipe dir. readme_only = tmp_path / "core" / "readme-only" diff --git a/tools/tests/test_validate_placement.py b/tools/tests/test_validate_placement.py new file mode 100644 index 0000000000..ec0664f8cf --- /dev/null +++ b/tools/tests/test_validate_placement.py @@ -0,0 +1,200 @@ +#!/usr/bin/env python3 +"""Unit tests for tools/validate_placement.py. + +`describe_violation` is pure string logic and gets a table. The scanning +layer is exercised against temporary trees, since the whole point of the +checker is what it finds on disk. +""" + +from pathlib import Path + +import pytest +import validate_placement as m + +MANIFEST = "manifest.yaml" + + +def _make_manifest(root: Path, rel_dir: str) -> Path: + """Create //manifest.yaml and return the manifest path.""" + directory = root / rel_dir + directory.mkdir(parents=True, exist_ok=True) + manifest = directory / MANIFEST + manifest.write_text("type: recipe\n", encoding="utf-8") + return manifest + + +# --------------------------------------------------------------------------- +# describe_violation — pure string logic +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "rel_parts", + [ + ["skills", "retail", "store-ops", MANIFEST], + ["skills", "hr", "onboarding", MANIFEST], + ["skills", "finance", "month-end-close", MANIFEST], + ], +) +def test_correctly_placed_returns_none(rel_parts): + assert m.describe_violation(rel_parts) is None + + +def test_flat_skill_is_rejected(): + """The case that matters: a solution dropped straight under skills/.""" + message = m.describe_violation(["skills", "foo", MANIFEST]) + assert message is not None + assert "no vertical" in message + assert "skills//foo" in message + + +def test_flat_skill_message_names_the_offending_dir(): + message = m.describe_violation(["skills", "store-ops", MANIFEST]) + assert "'skills/store-ops'" in message + assert "skills/retail/store-ops" in message + + +def test_manifest_at_root_of_skills_is_rejected(): + message = m.describe_violation(["skills", MANIFEST]) + assert message is not None + assert "no vertical" in message + + +def test_too_deep_is_rejected(): + message = m.describe_violation( + ["skills", "retail", "store-ops", "subproject", MANIFEST] + ) + assert message is not None + assert "too deeply" in message + assert "skills/retail/store-ops/subproject" in message + + +# --------------------------------------------------------------------------- +# find_manifests — scanning and pruning +# --------------------------------------------------------------------------- + + +def test_find_manifests_on_missing_root_is_empty(tmp_path): + assert m.find_manifests(tmp_path / "skills") == [] + + +def test_find_manifests_prunes_vendored_dirs(tmp_path): + _make_manifest(tmp_path, "skills/retail/store-ops") + _make_manifest(tmp_path, "skills/retail/store-ops/.venv/lib/pkg") + _make_manifest(tmp_path, "skills/retail/store-ops/node_modules/thing") + found = m.find_manifests(tmp_path / "skills") + assert [str(p.relative_to(tmp_path)) for p in found] == [ + f"skills/retail/store-ops/{MANIFEST}" + ] + + +# --------------------------------------------------------------------------- +# check_root — scanning plus classification +# --------------------------------------------------------------------------- + + +def test_check_root_accepts_valid_tree(tmp_path, capsys): + _make_manifest(tmp_path, "skills/retail/store-ops") + _make_manifest(tmp_path, "skills/hr/onboarding") + assert m.check_root("skills", repo_root=tmp_path) == [] + assert "::error" not in capsys.readouterr().out + + +def test_check_root_flags_flat_skill(tmp_path, capsys): + _make_manifest(tmp_path, "skills/retail/store-ops") + _make_manifest(tmp_path, "skills/oops") + errors = m.check_root("skills", repo_root=tmp_path) + assert len(errors) == 1 + out = capsys.readouterr().out + assert f"::error file=skills/oops/{MANIFEST}::" in out + + +def test_check_root_flags_every_offender(tmp_path): + _make_manifest(tmp_path, "skills/a") + _make_manifest(tmp_path, "skills/b") + _make_manifest(tmp_path, "skills/retail/ok") + assert len(m.check_root("skills", repo_root=tmp_path)) == 2 + + +def test_check_root_flags_too_deep(tmp_path): + _make_manifest(tmp_path, "skills/retail/store-ops/inner") + errors = m.check_root("skills", repo_root=tmp_path) + assert len(errors) == 1 + assert "too deeply" in errors[0] + + +def test_check_root_on_empty_root(tmp_path): + (tmp_path / "skills").mkdir() + assert m.check_root("skills", repo_root=tmp_path) == [] + + +# --------------------------------------------------------------------------- +# main — exit codes against the real repository +# --------------------------------------------------------------------------- + + +def test_main_passes_on_the_real_repo(): + """The repository must always be placement-clean on main.""" + assert m.main() == 0 + + +def test_main_tolerates_a_missing_skills_root(monkeypatch, tmp_path, capsys): + monkeypatch.setattr(m, "REPO_ROOT", tmp_path) + assert m.main() == 0 + assert "[SKIP]" in capsys.readouterr().out + + +def test_main_reports_failure(monkeypatch, tmp_path, capsys): + _make_manifest(tmp_path, "skills/oops") + monkeypatch.setattr(m, "REPO_ROOT", tmp_path) + assert m.main() == 1 + out = capsys.readouterr().out + assert "ACTION REQUIRED" in out + assert "1 misplaced recipe" in out + + +def test_main_narrows_to_the_given_scope(monkeypatch, tmp_path): + """A scope must not surface violations from elsewhere in the tree, or + `uv run validate ` would fail on an unrelated skill.""" + _make_manifest(tmp_path, "skills/oops") + _make_manifest(tmp_path, "skills/retail/store-ops") + monkeypatch.setattr(m, "REPO_ROOT", tmp_path) + assert m.main("skills/retail") == 0 + assert m.main("skills") == 1 + assert m.main() == 1 + + +def test_main_ignores_a_scope_outside_the_checked_roots( + monkeypatch, tmp_path, capsys +): + _make_manifest(tmp_path, "skills/oops") + monkeypatch.setattr(m, "REPO_ROOT", tmp_path) + assert m.main("core/python/foo") == 0 + assert "::error" not in capsys.readouterr().out + + +def test_registered_as_a_validate_subcommand(): + import validate + + assert "placement" in validate.SUBCOMMANDS + assert validate.SUBCOMMANDS["placement"][1] is m.main + assert "placement" in validate.VALID_SUBCOMMANDS + + +# --------------------------------------------------------------------------- +# Configuration guards +# --------------------------------------------------------------------------- + + +def test_checked_roots_tracks_the_shared_constant(): + import validate_manifest as vm + + assert set(m.CHECKED_ROOTS) == set(vm.NAMESPACE_REQUIRED_ROOTS) + assert "skills" in m.CHECKED_ROOTS + + +def test_core_and_contrib_are_not_checked_yet(): + """Flat core/ is still legal, so those roots stay out of scope + until that layout is retired.""" + assert "core" not in m.CHECKED_ROOTS + assert "contrib" not in m.CHECKED_ROOTS diff --git a/tools/tests/test_validate_readme.py b/tools/tests/test_validate_readme.py index 0cddc92650..ca82447592 100644 --- a/tools/tests/test_validate_readme.py +++ b/tools/tests/test_validate_readme.py @@ -114,16 +114,15 @@ def test_validate_readme_too_short(tmp_path): def test_validate_readme_missing_setup(tmp_path): - content = VALID_README.replace("## Setup", "## Configuration Details") - # "configuration" is in the keyword list, so swap to something not in the list - content = content.replace("## Configuration Details", "## Prerequisites Info Extra Words") + # _SETUP_KEYWORDS matches on word boundaries, and is broad — "setup", + # "prerequisites", "configuration", "installation", "requirements", + # "environment" and more all count as a setup section. The replacement + # heading has to dodge every one of them to reach the missing-setup + # path at all. + content = VALID_README.replace("## Setup", "## Overview of Dependencies") readme = _write(tmp_path / "README.md", content) errors = r.validate_readme(readme) - # "prerequisites" IS in the keyword list — use a heading that truly isn't - content2 = VALID_README.replace("## Setup", "## Overview of Dependencies") - readme2 = _write(tmp_path / "README2.md", content2) - errors2 = r.validate_readme(readme2) - assert any("setup" in e for e in errors2) + assert any("setup" in e for e in errors) def test_validate_readme_setup_synonym_prerequisites(tmp_path): @@ -240,7 +239,9 @@ def test_main_invalid_readme_returns_one(tmp_path, monkeypatch): assert r.main("core/bad") == 1 -def test_main_missing_readme_emits_github_annotation(tmp_path, monkeypatch, capsys): +def test_main_missing_readme_emits_github_annotation( + tmp_path, monkeypatch, capsys +): _make_recipe(tmp_path, "core/no-readme", readme=None) monkeypatch.setattr(m, "REPO_ROOT", tmp_path) monkeypatch.setattr(r, "REPO_ROOT", tmp_path) @@ -252,7 +253,9 @@ def test_main_missing_readme_emits_github_annotation(tmp_path, monkeypatch, caps assert "missing" in out -def test_main_invalid_readme_emits_github_annotation(tmp_path, monkeypatch, capsys): +def test_main_invalid_readme_emits_github_annotation( + tmp_path, monkeypatch, capsys +): _make_recipe(tmp_path, "core/bad", readme="too short\n") monkeypatch.setattr(m, "REPO_ROOT", tmp_path) monkeypatch.setattr(r, "REPO_ROOT", tmp_path) diff --git a/tools/tests/test_validate_structure.py b/tools/tests/test_validate_structure.py index 00c6175d76..f1052993c8 100644 --- a/tools/tests/test_validate_structure.py +++ b/tools/tests/test_validate_structure.py @@ -75,13 +75,23 @@ def _base_policy() -> dict: "by_root": { "core": ["AGENTS.md"], "contrib": [], - "skills": ["SKILL.md"], + "skills": ["SKILL.md", "EVAL.yaml"], }, "by_language": { "python": ["pyproject.toml", "uv.lock"], "java": [], }, - } + }, + "required_dirs": { + "always": [], + "by_root": { + "core": [], + "contrib": [], + "skills": ["scripts", "assets", "references", "tests/unit"], + }, + "by_language": {"python": [], "java": []}, + }, + "case_insensitive_files": ["EVAL.yaml"], } @@ -103,7 +113,7 @@ def test_required_files_for_contrib_python(): def test_required_files_for_skills_no_language(): files = m.required_files_for(_base_policy(), "skills", None) - assert files == ["README.md", "SKILL.md"] + assert files == ["README.md", "SKILL.md", "EVAL.yaml"] def test_required_files_for_unknown_root_and_language(): @@ -382,11 +392,16 @@ def test_check_size_and_count_large_tier_relaxes(tmp_path): def test_check_size_and_count_root_without_limits_skips(tmp_path): - # No `skills` section in recipe_size_limits → nothing to enforce. - recipe = _make_python_recipe(tmp_path, "skills/foo", include_agents=False) + # A root with no section in recipe_size_limits → nothing to enforce. + # Uses a fictional root because core, contrib and skills all have real + # limits in .github/policy.yml now, so none of them demonstrates this. + recipe = _make_python_recipe( + tmp_path, "playground/foo", include_agents=False + ) manifest = recipe / "manifest.yaml" assert ( - m.check_size_and_count(recipe, "skills", _size_policy(), manifest) == [] + m.check_size_and_count(recipe, "playground", _size_policy(), manifest) + == [] ) @@ -574,3 +589,204 @@ def test_main_returns_one_on_failure(fake_repo, capsys): def test_main_single_recipe_scope(fake_repo): _make_python_recipe(fake_repo, "core/only", include_agents=True) assert m.main("core/only") == 0 + + +# --------------------------------------------------------------------------- +# required_dirs_for — same three-way union as required_files_for +# --------------------------------------------------------------------------- + + +def test_required_dirs_for_skills(): + dirs = m.required_dirs_for(_base_policy(), "skills", "python") + assert dirs == ["scripts", "assets", "references", "tests/unit"] + + +def test_required_dirs_for_core_is_empty(): + assert m.required_dirs_for(_base_policy(), "core", "python") == [] + + +def test_required_dirs_for_missing_section_degrades(): + assert m.required_dirs_for({}, "skills", "python") == [] + + +def test_required_dirs_for_dedupes(): + policy = { + "required_dirs": { + "always": ["scripts"], + "by_root": {"skills": ["scripts", "assets"]}, + "by_language": {"python": ["assets"]}, + } + } + assert m.required_dirs_for(policy, "skills", "python") == [ + "scripts", + "assets", + ] + + +# --------------------------------------------------------------------------- +# Vertical skills — the full file + directory contract +# --------------------------------------------------------------------------- + +SKILL_MANIFEST = VALID_MANIFEST + + +def _make_skill(root: Path, rel: str = "skills/retail/store-ops") -> Path: + """A complete, valid Python vertical skill: every required file and + every required directory.""" + skill = root / rel + skill.mkdir(parents=True, exist_ok=True) + _write(skill / "manifest.yaml", SKILL_MANIFEST) + _write(skill / "README.md", "# skill\n") + _write(skill / "SKILL.md", "# installer\n") + _write(skill / "EVAL.yaml", "rubrics: []\n") + _write(skill / "pyproject.toml", "[project]\nname='x'\n") + _write(skill / "uv.lock", "# lockfile\n") + _write(skill / ".env.example", "FOO=1\n") + _write(skill / "tests" / "test_runnability.py", "def test(): pass\n") + for d in ("scripts", "assets", "references", "tests/unit"): + (skill / d).mkdir(parents=True, exist_ok=True) + return skill + + +def test_complete_skill_passes(isolated_repo): + skill = _make_skill(isolated_repo) + schema = vm.load_schema() + assert m.validate_recipe(skill, _full_policy(), schema) == [] + + +def test_skill_missing_a_required_dir_fails(isolated_repo): + skill = _make_skill(isolated_repo) + (skill / "assets").rmdir() + errors = m.validate_recipe(skill, _full_policy(), vm.load_schema()) + assert any("[required-dirs]" in e and "assets/" in e for e in errors) + + +def test_skill_missing_eval_yaml_fails(isolated_repo): + skill = _make_skill(isolated_repo) + (skill / "EVAL.yaml").unlink() + errors = m.validate_recipe(skill, _full_policy(), vm.load_schema()) + assert any("EVAL.yaml" in e for e in errors) + + +def test_empty_required_dirs_pass(isolated_repo): + """assets/ and references/ are legitimately empty for some skills.""" + skill = _make_skill(isolated_repo) + assert list((skill / "assets").iterdir()) == [] + assert ( + m.check_required_dirs(skill, "skills", "python", _full_policy()) == [] + ) + + +def test_required_dir_that_is_actually_a_file_is_reported_precisely( + isolated_repo, +): + """'missing' would send the author hunting for something that is + right there under the wrong kind.""" + skill = _make_skill(isolated_repo) + (skill / "scripts").rmdir() + _write(skill / "scripts", "oops\n") + errors = m.check_required_dirs(skill, "skills", "python", _full_policy()) + assert len(errors) == 1 + assert "exists but is a file" in errors[0] + + +# --------------------------------------------------------------------------- +# Case handling — must not depend on the host filesystem +# --------------------------------------------------------------------------- + + +def test_wrong_case_fails_for_a_strict_entry(isolated_repo): + """pyproject.toml is read by uv, which resolves it by exact name. + Accepting PyProject.toml would pass here and then break uv. This must + fail on macOS too, where the filesystem alone would accept it.""" + skill = _make_skill(isolated_repo) + (skill / "pyproject.toml").unlink() + _write(skill / "PyProject.toml", "[project]\nname='x'\n") + errors = m.check_required_files(skill, "skills", "python", _full_policy()) + assert any("pyproject.toml" in e and "missing" in e for e in errors) + + +def test_wrong_case_passes_for_eval_yaml_with_a_note(isolated_repo, capsys): + skill = _make_skill(isolated_repo) + (skill / "EVAL.yaml").unlink() + _write(skill / "eval.yaml", "rubrics: []\n") + assert ( + m.check_required_files(skill, "skills", "python", _full_policy()) == [] + ) + out = capsys.readouterr().out + assert "[NOTE]" in out + assert "eval.yaml" in out + assert "EVAL.yaml" in out + + +def test_exact_case_produces_no_note(isolated_repo, capsys): + skill = _make_skill(isolated_repo) + assert ( + m.check_required_files(skill, "skills", "python", _full_policy()) == [] + ) + assert "[NOTE]" not in capsys.readouterr().out + + +def test_find_entry_compares_names_in_python(tmp_path): + """Pins the platform-independence: the match is decided by comparing + names, never by asking the filesystem, so the verdict is the same on a + case-insensitive macOS volume and on Linux CI.""" + _write(tmp_path / "EVAL.yaml", "x") + assert m._find_entry(tmp_path, "EVAL.yaml", case_insensitive=False)[0] + assert m._find_entry(tmp_path, "eval.yaml", case_insensitive=False) == ( + None, + False, + ) + found, exact = m._find_entry(tmp_path, "eval.yaml", case_insensitive=True) + assert found is not None and exact is False + + +def test_find_entry_walks_nested_segments(tmp_path): + _write(tmp_path / "tests" / "unit" / "test_x.py", "x") + found, exact = m._find_entry(tmp_path, "tests/unit", case_insensitive=False) + assert found is not None and found.is_dir() and exact is True + assert m._find_entry(tmp_path, "tests/Unit", case_insensitive=False) == ( + None, + False, + ) + + +def test_case_insensitive_entries_reads_policy(): + assert m.case_insensitive_entries(_full_policy()) == {"EVAL.yaml"} + assert m.case_insensitive_entries({}) == set() + + +# --------------------------------------------------------------------------- +# Guards on the committed policy.yml +# --------------------------------------------------------------------------- + + +def test_committed_policy_declares_the_skill_contract(): + """A future edit must not silently drop part of the contract.""" + policy = m.load_policy() + files = m.required_files_for(policy, "skills", "python") + dirs = m.required_dirs_for(policy, "skills", "python") + for f in ( + "README.md", + "SKILL.md", + "EVAL.yaml", + "pyproject.toml", + "uv.lock", + "tests/test_runnability.py", + ): + assert f in files, f + for d in ("scripts", "assets", "references", "tests/unit"): + assert d in dirs, d + + +def test_committed_policy_keeps_tool_read_files_case_strict(): + """These are resolved by exact name by uv, pytest and our own tooling; + relaxing them would pass CI and then break the tool that reads them.""" + lenient = m.case_insensitive_entries(m.load_policy()) + for f in ( + "pyproject.toml", + "uv.lock", + "manifest.yaml", + "tests/test_runnability.py", + ): + assert f not in lenient, f diff --git a/tools/validate.py b/tools/validate.py index 1317f16931..552c69c2e2 100644 --- a/tools/validate.py +++ b/tools/validate.py @@ -37,6 +37,7 @@ import sys import validate_manifest +import validate_placement import validate_readme import validate_structure @@ -44,6 +45,12 @@ "manifest": ("Manifest validation", validate_manifest.main), "structure": ("Structure validation", validate_structure.main), "readme": ("README validation", validate_readme.main), + # Placement answers "is this recipe in the right folder", which the + # per-recipe checkers above cannot: they only ever see recipes the + # collector already found, and a misplaced one is missed by that + # collector. CI runs it as its own job; registering it here is what + # lets a contributor catch the problem locally before pushing. + "placement": ("Placement validation", validate_placement.main), } VALID_SUBCOMMANDS = [*SUBCOMMANDS, "all"] diff --git a/tools/validate_manifest.py b/tools/validate_manifest.py index 0ea8d705e4..85158dc6a2 100644 --- a/tools/validate_manifest.py +++ b/tools/validate_manifest.py @@ -51,6 +51,37 @@ LANGUAGE_NAMESPACE_DIRS = {"python", "java", "go", "typescript", "kotlin"} +# Roots whose second path component is ALWAYS a namespace, whatever it is +# called. core/ and contrib/ take an OPTIONAL language namespace, matched by +# name against LANGUAGE_NAMESPACE_DIRS. skills/ takes a MANDATORY vertical +# (retail/, hr/, finance/ …) whose name is free-form, so it can only be +# recognised by position: +# +# core// or core/ +# skills// +# +# The vertical surfaces ownership — it lets a team see its whole surface at +# a glance — so it is part of the layout rather than a value we enumerate. +NAMESPACE_REQUIRED_ROOTS = {"skills"} + + +def is_namespace_path(parts: list[str]) -> bool: + """True if these repo-relative path components identify a namespace + directory — a container of recipes — rather than a recipe itself. + + Depth matters: only the component directly under a recipe root can be a + namespace, which is what keeps `skills/retail` (a vertical) distinct + from `skills/retail/store-ops` (a solution). + """ + if len(parts) != 2: + return False + root, name = parts + if root not in RECIPE_ROOTS: + return False + if root in NAMESPACE_REQUIRED_ROOTS: + return True + return name in LANGUAGE_NAMESPACE_DIRS + def is_recipe_dir(path: Path) -> bool: """A recipe directory is any immediate subdirectory that contains @@ -147,10 +178,11 @@ def _collect_scoped_path(scope: str) -> list[Path]: if not target.exists(): print(f"[ERROR] Directory not found: {target}") sys.exit(1) - # Language namespace directory (e.g. core/python) — recurse one level. - # is_recipe_dir() already returns False for these, so we handle them - # explicitly here before the generic validity check below. - if target.name in LANGUAGE_NAMESPACE_DIRS: + # Namespace directory (e.g. core/python, skills/retail) — recurse one + # level. Matched on the scope's own components rather than just the + # basename, so `skills/retail` is a namespace while the solution beneath + # it, `skills/retail/store-ops`, is not. + if is_namespace_path(scope.strip("/").split("/")): recipe_dirs = sorted(c for c in target.iterdir() if is_recipe_dir(c)) if not recipe_dirs: print(f"[INFO] No recipe directories found under '{scope}/'.") @@ -165,10 +197,14 @@ def _collect_root(root_name: str) -> list[Path]: """Return the recipe directories directly under a top-level root. Recognised layouts: - / — flat - // — language-namespaced - - where ∈ LANGUAGE_NAMESPACE_DIRS. + / — flat (core/, contrib/) + // — language-namespaced (core/, contrib/) + skills// — vertical-namespaced (skills/) + + Under a NAMESPACE_REQUIRED_ROOTS root every child is a namespace, so a + solution placed directly at `skills/` is not collected here. + That misplacement is reported by tools/validate_placement.py rather + than silently validated at the wrong depth. """ root_path = REPO_ROOT / root_name if not root_path.exists(): @@ -179,8 +215,8 @@ def _collect_root(root_name: str) -> list[Path]: for p in sorted(root_path.iterdir()): if not p.is_dir(): continue - if p.name in LANGUAGE_NAMESPACE_DIRS: - # // + if is_namespace_path([root_name, p.name]): + # // or skills// recipe_dirs.extend( sorted(c for c in p.iterdir() if is_recipe_dir(c)) ) @@ -273,7 +309,8 @@ def main(scope: str | None = None) -> int: "\nFix the manifest.yaml file(s) listed above, then push again." "\n" "\nReference:" - "\n Schema: .github/schemas/manifest-schema.json" ) + "\n Schema: .github/schemas/manifest-schema.json" + ) return 1 checked = len(recipe_dirs) diff --git a/tools/validate_placement.py b/tools/validate_placement.py new file mode 100644 index 0000000000..50ed1a60a0 --- /dev/null +++ b/tools/validate_placement.py @@ -0,0 +1,172 @@ +#!/usr/bin/env python3 +""" +Validate that every recipe sits at the path its root requires. + +Recipes are located by their manifest.yaml, and the manifest's depth below +the recipe root tells you whether the recipe is in the right place: + + skills///manifest.yaml valid + skills//manifest.yaml too shallow — no vertical + skills///x/manifest.yaml too deep + +The vertical (retail/, hr/, finance/) is mandatory under skills/: it +surfaces ownership and lets a team reason about its whole surface at a +glance. A solution dropped directly under skills/ has no owning vertical, +so it is rejected. + +Scope: only roots listed in validate_manifest.NAMESPACE_REQUIRED_ROOTS are +checked. core/ and contrib/ still permit a flat / layout and +are deliberately left alone here — widen CHECKED_ROOTS once that layout is +retired. + +This is a whole-tree scan rather than a diff, so it also catches a +misplacement that arrives some other way (a rename, a bad merge). A missing +root is not an error: skills/ need not exist yet. + +Usage: + uv run python tools/validate_placement.py + +Exit codes: + 0 — every recipe is correctly placed + 1 — one or more recipes are misplaced +""" + +from __future__ import annotations + +import sys +from pathlib import Path + +import validate_manifest as vm + +REPO_ROOT = Path(__file__).parent.parent + +# Roots this checker enforces. Kept deliberately narrow: these are the roots +# where a namespace is mandatory, so the expected depth is unambiguous. +CHECKED_ROOTS = sorted(vm.NAMESPACE_REQUIRED_ROOTS) + +# Directory names never descended into while looking for manifests. +PRUNED_DIRS = {".git", ".venv", "node_modules", "__pycache__", ".ruff_cache"} + +# ///manifest.yaml +EXPECTED_PARTS = 4 + + +def find_manifests(root: Path) -> list[Path]: + """Every manifest.yaml beneath `root`, skipping vendored/build dirs.""" + if not root.is_dir(): + return [] + found: list[Path] = [] + for path in sorted(root.rglob(vm.MANIFEST_FILENAME)): + if any(part in PRUNED_DIRS for part in path.parts): + continue + found.append(path) + return found + + +def describe_violation(rel_parts: list[str]) -> str | None: + """Return an error message for a manifest path, or None if it is valid. + + `rel_parts` are the manifest's path components relative to the repo + root, e.g. ["skills", "retail", "store-ops", "manifest.yaml"]. + """ + if len(rel_parts) == EXPECTED_PARTS: + return None + + root = rel_parts[0] + recipe_dir = "/".join(rel_parts[:-1]) + expected = f"{root}///{vm.MANIFEST_FILENAME}" + + if len(rel_parts) < EXPECTED_PARTS: + solution = rel_parts[-2] if len(rel_parts) > 1 else "" + return ( + f"'{recipe_dir}' sits directly under '{root}/' with no vertical. " + f"Every {root}/ recipe must live at {expected} — move it to " + f"'{root}//{solution}' (e.g. " + f"'{root}/retail/{solution}'), choosing the vertical that owns " + f"it." + ) + + return ( + f"'{recipe_dir}' is nested too deeply. Every {root}/ recipe must " + f"live at {expected}, one directory below its vertical." + ) + + +def check_root( + root_name: str, + repo_root: Path = REPO_ROOT, + scope: str | None = None, +) -> list[str]: + """Return an error string for every misplaced recipe under a root. + + `scope`, when given, restricts reporting to manifests beneath that + repo-relative path, so `validate placement core/python/foo` cannot fail + on an unrelated problem elsewhere in the tree. + """ + errors: list[str] = [] + for manifest in find_manifests(repo_root / root_name): + rel = manifest.relative_to(repo_root) + if scope and not ( + str(rel) == scope or str(rel).startswith(f"{scope}/") + ): + continue + message = describe_violation(list(rel.parts)) + if message is None: + continue + print(f"::error file={rel}::{message}") + errors.append(message) + return errors + + +def main(scope: str | None = None) -> int: + """Check every root in CHECKED_ROOTS, or just the part under `scope`. + + Placement is a whole-tree property, so the useful invocation is the + unscoped one that CI runs. A scope is honoured anyway — it is what the + other `validate` subcommands accept, and narrowing keeps + `uv run validate ` from failing on an unrelated skill. + """ + prefix = scope.strip("/") if scope else None + all_errors: list[str] = [] + for root_name in CHECKED_ROOTS: + # A scope pointing outside this root (e.g. core/python/foo) means + # there is nothing here for it to say. + if prefix and not ( + prefix == root_name or prefix.startswith(f"{root_name}/") + ): + continue + root_path = REPO_ROOT / root_name + if not root_path.is_dir(): + print(f"[SKIP] '{root_name}/' does not exist.") + continue + # Pass REPO_ROOT explicitly: check_root's default argument is bound + # at import time, so relying on it would ignore any later rebinding + # of the module global (which is how the tests point at a fixture). + errors = check_root(root_name, repo_root=REPO_ROOT, scope=prefix) + if errors: + print(f"\n[FAIL] Misplaced recipes under '{root_name}/':\n") + for message in errors: + print(f" - {message}") + all_errors.extend(errors) + else: + print(f"[PASS] All '{root_name}/' recipes are correctly placed.") + + if not all_errors: + return 0 + + count = len(all_errors) + noun = "recipe" if count == 1 else "recipes" + print("") + print("=" * 60) + print(f" ACTION REQUIRED: {count} misplaced {noun}") + print("=" * 60) + print("") + print("Move each recipe to the path shown above, then re-run:") + print("") + print(" uv run python tools/validate_placement.py") + print("") + return 1 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tools/validate_readme.py b/tools/validate_readme.py index 4947dfc139..74d1bce1c3 100644 --- a/tools/validate_readme.py +++ b/tools/validate_readme.py @@ -165,10 +165,7 @@ def main(scope: str | None = None) -> int: if missing: passed = False - print( - "\n[FAIL] Missing README.md in the following recipe" - " directories:" - ) + print("\n[FAIL] Missing README.md in the following recipe directories:") for d in missing: print(f" - {d}/") print( diff --git a/tools/validate_structure.py b/tools/validate_structure.py index 26adab55b9..9253c6350b 100644 --- a/tools/validate_structure.py +++ b/tools/validate_structure.py @@ -17,6 +17,14 @@ policy.required_files.by_language[]. manifest.yaml itself is NOT listed in policy.required_files; check 1 is authoritative for it, and duplicating the rule would double-report. + 6. Required directories present — the same three-way union over + policy.required_dirs. Vertical skills use this for scripts/, + assets/, references/ and tests/unit/. An empty directory passes. + +Name matching for checks 5 and 6 is done in Python rather than by asking +the filesystem, so a case-mismatched file fails identically on macOS and +on Linux CI. Entries listed in policy.case_insensitive_files may be +spelled in any case and report an advisory note instead. Language detection: manifest.language is the ONLY source of truth. Path convention (e.g. core/python/foo/) carries no meaning to this checker. @@ -88,30 +96,62 @@ def load_policy() -> dict: return yaml.safe_load(f) or {} -def required_files_for( - policy: dict, root: str, language: str | None +def _resolve_required( + policy: dict, section: str, root: str, language: str | None ) -> list[str]: """Union of `always`, `by_root[root]`, and `by_language[language]` - from policy.required_files. Missing sections and missing keys degrade - silently to an empty list so partial policy configs remain usable - (e.g. `skills:` not yet defined).""" - rf = policy.get("required_files") or {} - files: list[str] = [] - files.extend(rf.get("always") or []) - files.extend((rf.get("by_root") or {}).get(root) or []) + from the named policy section. Missing sections and missing keys + degrade silently to an empty list so partial policy configs remain + usable (e.g. a root with no entries yet).""" + config = policy.get(section) or {} + entries: list[str] = [] + entries.extend(config.get("always") or []) + entries.extend((config.get("by_root") or {}).get(root) or []) if language: - files.extend((rf.get("by_language") or {}).get(language) or []) - # Preserve order but drop duplicates — a file could be listed under + entries.extend((config.get("by_language") or {}).get(language) or []) + # Preserve order but drop duplicates — an entry could be listed under # multiple sources without meaning it's required twice. seen: set[str] = set() deduped: list[str] = [] - for f in files: - if f not in seen: - seen.add(f) - deduped.append(f) + for entry in entries: + if entry not in seen: + seen.add(entry) + deduped.append(entry) return deduped +def required_files_for( + policy: dict, root: str, language: str | None +) -> list[str]: + """Files every recipe under this root/language must ship.""" + return _resolve_required(policy, "required_files", root, language) + + +def required_dirs_for( + policy: dict, root: str, language: str | None +) -> list[str]: + """Directories every recipe under this root/language must ship. + + Separate from required_files because the two need different existence + tests; a `scripts` entry in required_files could never be satisfied by + a directory. + """ + return _resolve_required(policy, "required_dirs", root, language) + + +def case_insensitive_entries(policy: dict) -> set[str]: + """Required entries whose name may be spelled in any case. + + Opt-in per entry, deliberately. Most required files are read by other + tools that demand an exact name — uv reads `pyproject.toml` and + `uv.lock`, pytest collects `tests/test_runnability.py`, and our own + tooling globs `manifest.yaml`. Accepting `PyProject.toml` would pass + this check and then break uv, which is a worse failure than the one + the leniency prevents. Only files nothing else parses belong here. + """ + return {str(e) for e in (policy.get("case_insensitive_files") or [])} + + # =========================================================================== # Root & language detection # =========================================================================== @@ -296,6 +336,49 @@ def check_size_and_count( return errors +def _find_entry( + recipe_dir: Path, rel: str, *, case_insensitive: bool +) -> tuple[Path | None, bool]: + """Resolve a recipe-relative path by comparing names in Python. + + Returns (path, exact). `path` is None when nothing matched; `exact` + reports whether the match used the requested spelling. + + Deliberately does NOT use Path.is_file()/is_dir() to test for + existence: those delegate to the filesystem, and macOS is normally + case-insensitive, so `PyProject.toml` would satisfy `pyproject.toml` + locally and then fail on Linux CI. Comparing names here makes the + verdict identical on every platform, and makes leniency an explicit + per-entry decision rather than a property of the contributor's laptop. + """ + current = recipe_dir + exact = True + for segment in Path(rel).parts: + if not current.is_dir(): + return None, False + children = sorted(current.iterdir()) + match = next((c for c in children if c.name == segment), None) + if match is None and case_insensitive: + lowered = segment.lower() + match = next( + (c for c in children if c.name.lower() == lowered), None + ) + if match is not None: + exact = False + if match is None: + return None, False + current = match + return current, exact + + +def _note_inexact(rel: str, found: Path) -> None: + print( + f"[NOTE] Required entry '{rel}' matched as '{found.name}'. " + f"Accepted, but '{rel}' is the canonical spelling — renaming keeps " + "the tree consistent for tools that look for the exact name." + ) + + def check_required_files( recipe_dir: Path, root: str, language: str | None, policy: dict ) -> list[str]: @@ -303,11 +386,46 @@ def check_required_files( Paths are recipe-relative and may include subdirectories (e.g. tests/test_runnability.py).""" required = required_files_for(policy, root, language) + lenient = case_insensitive_entries(policy) errors: list[str] = [] for rel in required: - target = recipe_dir / rel - if not target.is_file(): + found, exact = _find_entry( + recipe_dir, rel, case_insensitive=rel in lenient + ) + if found is None: errors.append(f"Required file '{rel}' is missing.") + elif not found.is_file(): + errors.append(f"Required file '{rel}' exists but is not a file.") + elif not exact: + _note_inexact(rel, found) + return errors + + +def check_required_dirs( + recipe_dir: Path, root: str, language: str | None, policy: dict +) -> list[str]: + """Every path in the union list must exist as a directory. + + An empty directory passes: some required folders (assets/, + references/) legitimately have nothing in them for a given recipe, + and git cannot commit an empty directory anyway, so in practice they + carry a .gitkeep. + """ + required = required_dirs_for(policy, root, language) + lenient = case_insensitive_entries(policy) + errors: list[str] = [] + for rel in required: + found, exact = _find_entry( + recipe_dir, rel, case_insensitive=rel in lenient + ) + if found is None: + errors.append(f"Required directory '{rel}/' is missing.") + elif not found.is_dir(): + # Reporting this as "missing" would send the author hunting + # for something that is right there under the wrong kind. + errors.append(f"Required directory '{rel}/' exists but is a file.") + elif not exact: + _note_inexact(rel, found) return errors @@ -363,6 +481,12 @@ def validate_recipe(recipe_dir: Path, policy: dict, schema: dict) -> list[str]: for err in check_required_files(recipe_dir, root, language, policy): errors.append(f"[required-files] {err}") + # Check 6: required directories. Same union, different existence test — + # a vertical skill must ship scripts/, assets/, references/ and + # tests/unit/, none of which required_files could express. + for err in check_required_dirs(recipe_dir, root, language, policy): + errors.append(f"[required-dirs] {err}") + return errors diff --git a/typescript/README.md b/typescript/README.md index 4e6a00e64e..39dfe2db58 100644 --- a/typescript/README.md +++ b/typescript/README.md @@ -1,5 +1,18 @@ # Agent Development Kit (ADK) TypeScript Samples +> [!IMPORTANT] +> **This folder is retired.** `typescript/agents/` no longer accepts new +> recipes, or changes to the recipes already here. Recipes now live in +> **`contrib/typescript/`**. +> +> - **Contributing a new recipe?** Start with the +> [recipe checklist](../docs/recipe-checklist.md). +> - **Already have a recipe here?** Move it to `contrib/typescript/` and +> follow the same checklist. See the +> [contributor guide](../docs/README.md). +> +> Pull requests that add or modify files under `typescript/agents/` fail CI. + [![License](https://img.shields.io/badge/License-Apache_2.0-blue.svg)](LICENSE) Agent Development Kit Logo