Skip to content

feat(ci): enforce recipe placement, support vertical skills, and fix silent validation gates - #2354

Merged
happyhuman merged 5 commits into
mainfrom
fix-more-issues
Jul 29, 2026
Merged

feat(ci): enforce recipe placement, support vertical skills, and fix silent validation gates#2354
happyhuman merged 5 commits into
mainfrom
fix-more-issues

Conversation

@happyhuman

Copy link
Copy Markdown
Collaborator

Three related fixes to recipe validation. Each closes a case where CI either said nothing, said something false, or checked the wrong directory.

1. Vertical skills — skills/<vertical>/<solution>/

Vertical skills use a vertical (retail/, hr/, finance/) where core/ and contrib/ use a language. The tooling had a single concept for that path slot, so it read skills/retail/store-ops as recipe skills/retail — validating the vertical folder and never seeing the solution at all.

  • NAMESPACE_REQUIRED_ROOTS + is_namespace_path() teach the collectors that skills/ namespaces by position, not by name
  • New tools/validate_placement.py + check-recipe-placement job rejects a solution dropped directly under skills/, which the collectors otherwise skip silently
  • EVAL.yaml added to required files; scripts/, assets/, references/, tests/unit/ required via a new required_dirs policy section, since required_files cannot express a directory. Empty directories pass.
  • skills/ gets the same size limits as contrib/, plus a CODEOWNERS entry

Validated against a real skill (retail-product-search): the required shape matches what it already ships, and the checks correctly flag its 5 missing pieces.

2. Retired legacy folders

python/agents/, java/agents/, go/agents/, kotlin/agents/, typescript/agents/ are closed. A PR that adds or modifies anything there now fails with the destination spelled out:

python/agents/blog-writer is in a retired folder and no longer accepts changes. Move this recipe to contrib/python/blog-writer.

Deletions and renames are exempt (--diff-filter=AM), so migrating a recipe out is never blocked. Bots are exempt; maintainers can override with an allow-legacy-path label. The five legacy READMEs get a deprecation banner — they currently read as live, welcoming guides, which is why the PRs keep arriving.

Heads-up: 53 open PRs will go red on merge. That's the intent — they currently wait days for a manual "wrong folder" reply.

3. Dependabot gate told the truth

The generated dependabot.yml claimed it was "regenerated automatically on every push to main". It isn't — the sync workflow is workflow_dispatch-only, and its own comments explain why. The header now states the real contract, and --check prints a diff of the missing entry instead of a dead end.

Also fixed along the way

  • Case matching no longer depends on the host filesystem. is_file() is case-insensitive on macOS, so PyProject.toml passed locally and failed on Linux CI. Names are now compared in Python. EVAL.yaml opts into leniency explicitly; files other tools resolve by exact name (pyproject.toml, uv.lock, manifest.yaml, tests/test_runnability.py) never can.
  • scaffold-python-recipe rejected contrib/python — the documented home for new Python recipes — because of a stale CLI allow-list.
  • Naming collision settled: repo skills (.agents/skills/, build this repo) vs vertical skills (skills/, shipped to users). Both now defined in the handbook glossary.

Verification

  • 457 tests pass, up from 340 on main
  • ruff check / ruff format clean across tools/ and .github/scripts/
  • All four validators green, dependabot freshness gate green, uv lock --check in sync
  • Migration exemption verified with real git: a git mv of a recipe out of a retired folder produces R100 and passes

After merge

Two things CI can't do itself:

  1. Add "Check for work in retired folders" and "Check recipe placement" to branch protection's required checks, or they're red X's that can still be merged past
  2. Close the 8 zombie Dependabot PRs for retired folders — nothing will update or close them now that they're out of the generated config

Known gaps

  • Placement checking is scoped to skills/ until core/rag-agent-search and core/rag-vector-search are deleted; widening it afterwards is a one-line change
  • A recipe dropped somewhere new with no manifest.yaml is still invisible
  • Whether assets/design-spec.md should be a required file is still open

…tract

Vertical skills use a vertical (retail/, hr/, finance/) where core/ and
contrib/ use a language. The tooling had one concept for that path slot,
so it read skills/retail/store-ops as recipe "skills/retail" — validating
the vertical folder and never seeing the solution at all.

- Teach the collectors that skills/ always namespaces by position, not by
  name (NAMESPACE_REQUIRED_ROOTS + is_namespace_path).
- Add tools/validate_placement.py and a CI job to reject a solution placed
  directly under skills/, which the collectors otherwise skip silently.
- Require EVAL.yaml, plus scripts/, assets/, references/ and tests/unit/
  via a new required_dirs policy section (required_files cannot express a
  directory). Empty directories pass.
- Match required names in Python rather than via is_file()/is_dir(): the
  filesystem answered case-insensitively on macOS, so a recipe could pass
  locally and fail on Linux CI. EVAL.yaml opts into leniency explicitly;
  files that other tools resolve by exact name must never do so.
- Give skills/ the same size limits as contrib/, and a CODEOWNERS entry.

Also settles the naming collision between the two things called "skills":
repo skills (.agents/skills/, used to build this repo) and vertical skills
(skills/, shipped to users). Updates the repo skills' own stale path
references, and fixes a scaffold bug where contrib/python — the documented
home for new Python recipes — was rejected by the CLI allow-list.
@happyhuman happyhuman self-assigned this Jul 29, 2026
Three failures on PR #2354:

- zizmor `unpinned-uses` (12 findings, a mandatory check). The new jobs
  used tag refs, and touching two sibling workflows pulled their existing
  tag refs into the changed-file scan. All 12 now use the SHAs already
  canonical elsewhere in the repo.
- dependabot.yml was missing an entry for contrib/python/market-research-agent,
  which landed on main while this branch was open. Merged main and
  regenerated: 16 entries.
- core/python/ambient-expense-agent/frontend/uv.lock was stale. Pre-existing,
  surfaced because the python-dependency-policy.yml change on this branch
  triggers a full scan rather than a diff-scoped one. Regenerated; the only
  delta is registry URL normalisation, no version changes.
@happyhuman
happyhuman requested a review from eliasecchig as a code owner July 29, 2026 18:58
@happyhuman
happyhuman merged commit 4283805 into main Jul 29, 2026
24 of 25 checks passed
@happyhuman
happyhuman deleted the fix-more-issues branch July 29, 2026 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant