Refactor recipe CI into language-agnostic structure workflow (+ oauth recipe) - #2248
Merged
Conversation
…rkflow
Prepares CI for the multi-language, multi-folder future (java/go/kotlin/
typescript recipes, plus the planned skills/ folder) by extracting every
check that isn't Python-specific into its own workflow and driving the
requirement matrix from .github/policy.yml.
New:
- validate-recipe-structure.yml — runs manifest presence + schema,
folder name, folder size + file count, and the required-files matrix
for every recipe under core/, contrib/, and (once it lands) skills/.
- policy.yml required_files: block — declarative {always, by_root,
by_language} matrix. Adding a language or a folder rule (e.g. SKILL.md
for skills/) is now a YAML edit, not a workflow diff.
- tools/validate_structure.py + tools/affected_recipes.py — the checker
and shared "changed files → recipe dirs" mapper, both testable and
runnable locally via `uv run validate structure [<recipe>]`. The
affected-recipe helper supports --language for path-first filtering
(namespaced dirs authoritative, flat recipes read manifest.language).
- 72 new tests covering the checker, the mapper, and the language filter.
Changed:
- python-validate-recipe.yml shrunk from 666 → 305 lines. Every check
that's not Python-specific was removed (folder name, size, file
count, required files, ~180 lines of bash for affected-recipe
detection). What remains: .env.example notice keys, env-var extraction
(check_env_vars.py), no-local-ruff-config, [project] metadata
(check_recipe_pyproject.py), hardcoded-model-name notice. Recipe
detection now delegates to `affected_recipes.py --language python`.
- validate_manifest.py: RECIPE_ROOTS now includes "skills"; the tool
gracefully skips the root when the directory is missing.
- docs/recipe-guidelines.md: required-files tables restructured to match
the new matrix (every / by-folder / by-language); added skills/ and
AGENTS.md; documented the new `uv run validate structure` command and
the two CI workflows.
Deleted:
- validate-manifest.yml — absorbed into the structure workflow (manifest
presence + schema are now checks 1 & 2 there).
No behavioural regression for existing recipes: all 5 committed recipes
still pass every check under both workflows; the "AGENTS.md required for
core/" generalisation (previously core/python/* only) is a no-op today
because both flat core/ recipes already carry AGENTS.md.
Follow-up to the previous commit, addressing five review points: 1. Stale comment in validate-recipe-structure.yml claiming the workflow "runs alongside validate-manifest.yml" — that file was deleted in the same change. Rewrote the header to state the actual role. 2. Full-scan collector silently skipped container-namespaced recipes (e.g. core/harnesses/python/<recipe>): _collect_root only recursed two levels. Added a third-level branch for CONTAINER_DIRS so the full-repo scan matches what affected_recipes.py accepts on the PR-scoped path. Consolidated CONTAINER_DIRS into validate_manifest.py as the single source of truth; affected_recipes.py now imports it instead of maintaining its own copy. Added two regression tests. 3. Stale "same install step validate-manifest.yml uses" comment in the structure workflow's uv sync step — rewritten to describe what the step actually needs (PyYAML + jsonschema). 4. Stale policy.yml comments claiming size/naming/required-files were enforced by python-validate-recipe.yml (they moved) and calling the structure workflow "upcoming" (it's live). Updated to point at validate-recipe-structure.yml. 5. validate-recipe-structure.yml's setup-uv step did not pin python-version, unlike python-validate-recipe.yml. Pinned to "3.11" for reproducibility and consistency across workflows. Review point #6 (env-var audit skipped when .env.example is missing) is pre-existing behaviour, not introduced by this branch, and is left alone as noted in the review. All 134 tests pass (2 new).
The harnesses/ container layout (`core/harnesses/python/<recipe>/`) was an old idea that we're not doing anymore. Recipes live at either `<root>/<language>/<recipe>` or the flat `<root>/<recipe>`; nothing else. Reverses the container support added in 4644b9a (which was itself a review fix for a "silently skipped" bug) — that support was covering a layout we don't actually want. Changes: - validate_manifest.py: dropped CONTAINER_DIRS; simplified _collect_root to the two supported layouts (flat + language-namespaced). Cleaned up the is_recipe_dir comment; kept the "all children are language namespaces → not a recipe" defence-in-depth clause with a note that it now only guards against accidental structures. - affected_recipes.py: dropped the container branches from recipe_dir_for and recipe_language_namespace; import of CONTAINER_DIRS removed. Docstrings updated. - Tests: removed the 2 container-namespace regression tests from test_validate_manifest.py and the ~9 container-related cases from test_affected_recipes.py. 123 tests pass (was 134). All 5 committed recipes still validate.
Follow-up to 4d02994. Six issues from the round-2 review: 1. (Critical) `uv run --no-project` broke `tools/affected_recipes.py` in CI — it imports PyYAML and the `validate_manifest` package, both supplied by the project venv. `--no-project` creates an ephemeral env without them, so every recipe-only PR would fail at the "Detect affected recipe directories" step with `ModuleNotFoundError: No module named 'yaml'`. Dropped `--no-project` from all three invocations across validate-recipe-structure.yml (1) and python-validate-recipe.yml (2). Verified by running the broken command locally — it reproduces the crash. 2. Documented the intentional broadening of the AGENTS.md requirement from `core/python/*` to ALL `core/*` recipes in policy.yml. Behaviour unchanged — this is the rule the user explicitly asked for; the comment just makes it explicit for future maintainers who might otherwise treat it as an oversight. 3. python-validate-recipe.yml's full-scan step used `find core contrib`, so Python recipes under `skills/` were never picked up during workflow_dispatch or infra-change runs (structural checks ran, Python-specific checks were silently skipped). Added `skills` to the find list; missing roots still tolerated silently via 2>/dev/null. 4. validate_structure.check_size_and_count silently fell back from `large` to `default` tier when the manifest set `large: true` but the root had no `large` block in policy.yml (via a stealthy `.get(tier) or .get("default")` chain). Rewrote to fail loudly with a clear "add a large tier or drop large: true" message. Two regression tests: (a) the failure fires when appropriate; (b) a root with neither tier plus a default (non-large) request is still a no-op. 5. Added a test for the language filter on `skills/python/<recipe>` to lock the invariant that python-validate-recipe.yml's full scan (which now enumerates `skills/` per fix 3) actually sees such recipes during workflow_dispatch. 6. recipe_dir_for("core/") returned "core/" — a bogus candidate that the existence filter would happily accept because core/ IS a real directory. Added an explicit early return of None when the second path component is empty; test that previously asserted the broken behaviour now asserts the correct one. All 127 tests pass (+4 new). Real committed recipes still validate under both `uv run validate manifest` and `uv run validate structure`.
…ools, and evaluation infrastructure
…d integration test configuration
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three things in one PR, main change first:
validate-recipe-structure.ymland a slimmed-down Python-onlypython-validate-recipe.yml. Required-files matrix moves into.github/policy.ymlas declarative config. Preps the tree for multi-language recipes (java/go/kotlin/typescript) and the plannedskills/folder.core/python/oauth-user-consent-flow— an ADK agent deployed on Agent Runtime with OAuth 2.0, reads Google Drive files on behalf of authenticated users..github/workflows/now has a 20–30 word summary comment right aftername:.The CI refactor (the main change)
Before:
python-validate-recipe.ymlowned every check for Python recipes — structural (folder name, size, file count, required files, manifest schema) and Python-specific (env-var extraction,[project]metadata, no-local-ruff, model-name notice). 666 lines of bash. Adding a new language would have meant duplicating most of it.After:
validate-recipe-structure.yml(new) — language-agnostic; runs manifest presence + schema, folder name, folder size + file count, and the required-files matrix for every recipe undercore/,contrib/, andskills/.python-validate-recipe.yml(kept, shrunk 666 → 305 lines) — Python-only:.env.exampleenv-key notice, AST-based env-var extraction, no-local-ruff-config,[project]metadata (name matches folder,requires-pythonfloor, description matches manifest), hardcoded-model-name notice.validate-manifest.yml(deleted) — absorbed into the structure workflow.Language detection now uses
manifest.languageas the single source of truth; path convention is not consulted by the structural checker.The required-files matrix
Now declarative in
.github/policy.yml:Adding a language or a folder rule (e.g.
SKILL.mdforskills/) is a one-line YAML edit — no workflow diff, no bash surgery.New shared tooling
tools/validate_structure.py— the structural checker; runnable locally viauv run validate structure [<recipe>]oruv run validate(which now runs both manifest + structure).tools/affected_recipes.py— shared "changed files → recipe directories" mapper. Handles flat (<root>/<recipe>) and language-namespaced (<root>/<language>/<recipe>) layouts. Supports--language <lang>filter with path-first semantics (namespaced dirs authoritative, flat recipes readmanifest.language). Both existing workflows now use this instead of duplicating the mapping in bash.tools/).Adding a new language later
For java/go/kotlin/typescript, the recipe author only needs to:
required_files.by_languageinpolicy.yml.<lang>-validate-recipe.ymlfor language-specific rules only.Structural validation is already covered.
The new recipe
core/python/oauth-user-consent-flow— production-ready ADK agent, deploys to Agent Runtime, reads Google Drive files with OAuth 2.0 consent. Works both locally via ADK Web UI and in production via Gemini Enterprise. Comes with unit + integration + eval tests and a runnability test.Workflow docs pass
Every workflow file under
.github/workflows/now has a 20–30 word summary comment directly aftername:. The two workflows I extensively touched had detailed 120-word comments compressed to the same style, and all other workflows got one added (they had none before). Consistent, greppable, and cheap to keep updated.Docs
docs/recipe-guidelines.mdrestructured:skills/andAGENTS.md(previously omitted).uv run validate structureanduv run validatecommands.Verified
tools/tests pass locally.uv run validate manifestanduv run validate structure.git diff→affected_recipes.py→ per-recipe validation works on real recipe paths.No behavioural regression
The AGENTS.md broadening (previously
core/python/*only, now allcore/*) is a no-op today because both existing flatcore/recipes already carryAGENTS.md. Futurecore/java/foo/etc. will need it too — this is intentional and now documented inpolicy.yml.