diff --git a/pyproject.toml b/pyproject.toml index d269ba840be2..bd696d191fd2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -301,28 +301,22 @@ hermes-acp = "acp_adapter.entry:main" [tool.setuptools] py-modules = ["run_agent", "model_tools", "toolsets", "batch_runner", "trajectory_compressor", "toolset_distributions", "cli", "hermes_bootstrap", "hermes_constants", "hermes_state", "hermes_time", "hermes_logging", "utils", "mcp_serve"] -[tool.setuptools.data-files] -# i18n catalogs. locales/ is a bare data directory (no __init__.py), so it is -# neither a package (packages.find) nor package-data (which attaches to a -# package). data-files ships it in the wheel; MANIFEST.in `graft locales` -# ships it in the sdist. Without this, sealed installs (pip wheel, Nix store -# venv) drop the catalogs and gateway/CLI commands surface raw i18n keys like -# `gateway.reset.header_default` (#27632, #35374, #23943). -locales = ["locales/*.yaml"] -# Shipped MCP catalog (optional-mcps//manifest.yaml). Same bare-data-dir -# case as locales: data-files ships it in the wheel, `graft optional-mcps` in -# MANIFEST.in ships it in the sdist. Without this, `hermes mcp catalog` and the -# dashboard catalog screen come up empty on packaged installs even though the -# manifests exist in the repo (hermes_cli/mcp_catalog.py:_catalog_root resolves -# the packaged dir; list_catalog() returns [] when it's missing). +# NOTE: there is intentionally NO [tool.setuptools.data-files] table here. # -# data-files flattens every glob match into its single target dir, so each -# catalog entry needs its OWN target to preserve the per-entry directory the -# catalog iterates over (a shared `optional-mcps/*/*` glob would collapse all -# manifests into one colliding optional-mcps/manifest.yaml). One target per -# entry; tests/test_packaging_metadata.py enforces an entry per optional-mcps/. -"optional-mcps/linear" = ["optional-mcps/linear/manifest.yaml"] -"optional-mcps/n8n" = ["optional-mcps/n8n/manifest.yaml"] +# Bundled non-package data dirs (skills/, optional-skills/, locales/, +# optional-mcps/) are bare data directories — no __init__.py, so they are +# neither packages (packages.find) nor package-data (which attaches to a +# package). They ship in the wheel via setuptools `data_files`, generated in +# setup.py:bundled_data_files(), and in the sdist via MANIFEST.in `graft`. +# +# data_files can be declared in EITHER this table OR setup.py, never merged: +# whichever pyproject declares wins and silently drops setup.py's list. The +# skills/ and optional-skills/ trees are too deep and change too often to +# enumerate statically here (data_files also flattens each glob into one target +# dir, so a nested tree needs one target per directory), so ALL four dirs are +# generated dynamically in setup.py. Reintroducing this table would shadow +# setup.py and the wheel would ship zero skills again — see +# tests/test_packaging_metadata.py and CONTRIBUTING.md (#27632, #35374, #23943). [tool.setuptools.package-data] hermes_cli = ["web_dist/**/*", "tui_dist/**/*", "scripts/install.sh", "scripts/install.ps1"] diff --git a/setup.py b/setup.py index 6e3e8c4272e8..4f8ca9445673 100644 --- a/setup.py +++ b/setup.py @@ -12,6 +12,37 @@ REPO_ROOT = Path(__file__).parent.resolve() +# Directory names that never carry shippable bundled-data content: VCS, +# virtualenv / dependency trees, byte-code and test caches, and the skills +# ``index-cache`` (a runtime download cache, also filtered by +# nix/hermes-agent.nix). Mirrors agent.skill_utils.EXCLUDED_SKILL_DIRS — kept +# as a literal so setup.py stays import-free at build time — plus +# ``index-cache``. NOTE: skill *support* dirs (references/templates/assets/ +# scripts) are deliberately NOT excluded; they are real skill payload that +# tools/skills_sync.py copies into ~/.hermes/skills/ on seed. +_EXCLUDED_DATA_DIRS = frozenset( + { + ".git", + ".github", + ".hub", + ".archive", + ".venv", + "venv", + "node_modules", + "site-packages", + "__pycache__", + ".tox", + ".nox", + ".pytest_cache", + ".mypy_cache", + ".ruff_cache", + "index-cache", + } +) +_EXCLUDED_DATA_SUFFIXES = frozenset({".pyc", ".pyo"}) +_EXCLUDED_DATA_NAMES = frozenset({".DS_Store", "Thumbs.db"}) + + def _source_tree_is_writable() -> bool: probe = REPO_ROOT / ".setuptools-write-probe" try: @@ -64,24 +95,67 @@ def finalize_options(self) -> None: super().finalize_options() +def _is_shippable(rel_path: Path) -> bool: + """True when *rel_path* (relative to REPO_ROOT) is real bundled-data content.""" + if _EXCLUDED_DATA_DIRS.intersection(rel_path.parts): + return False + if rel_path.suffix.lower() in _EXCLUDED_DATA_SUFFIXES: + return False + if rel_path.name in _EXCLUDED_DATA_NAMES: + return False + return True + + def _data_file_tree(root_name: str) -> list[tuple[str, list[str]]]: + """Map every file under *root_name* to a setuptools ``data_files`` entry. + + Returns ``(target_dir, [source_files])`` tuples with the on-disk directory + layout preserved one-directory-per-target. setuptools ``data_files`` + FLATTENS every glob match into its single target dir, so a nested tree must + be enumerated per directory or the structure collapses — skills are seeded + by category/skill path (tools/skills_sync.py), so a flattened + ``skills/*`` glob would corrupt the install. Paths are emitted as POSIX + (forward slash) so the wheel is correct even when built on Windows, where + ``str(Path(...))`` would otherwise bake in backslash targets. + """ root = REPO_ROOT / root_name + if not root.is_dir(): + return [] grouped: defaultdict[str, list[str]] = defaultdict(list) for path in sorted(root.rglob("*")): if not path.is_file(): continue rel_path = path.relative_to(REPO_ROOT) - grouped[str(rel_path.parent)].append(str(rel_path)) + if not _is_shippable(rel_path): + continue + grouped[rel_path.parent.as_posix()].append(rel_path.as_posix()) return sorted(grouped.items()) +def bundled_data_files() -> list[tuple[str, list[str]]]: + """All bundled non-package data shipped in the wheel's data scheme. + + setuptools resolves ``data_files`` from EITHER pyproject.toml's + ``[tool.setuptools.data-files]`` table OR setup.py — never a merge of both. + Whichever the pyproject table declares wins and silently drops setup.py's + list (this clobbered the skills shipping for two releases). The skills and + optional-skills trees are too deep and change too often to enumerate + statically in TOML, so ALL bundled-data dirs are generated here in one + place. Do NOT reintroduce ``[tool.setuptools.data-files]`` in pyproject.toml + or it will shadow this and the wheel will ship zero skills again. + """ + return [ + *_data_file_tree("skills"), + *_data_file_tree("optional-skills"), + *_data_file_tree("locales"), + *_data_file_tree("optional-mcps"), + ] + + setup( cmdclass={ "build": ReadOnlySourceBuild, "egg_info": ReadOnlySourceEggInfo, }, - data_files=[ - *_data_file_tree("skills"), - *_data_file_tree("optional-skills"), - ] + data_files=bundled_data_files(), ) diff --git a/tests/test_packaging_metadata.py b/tests/test_packaging_metadata.py index 5499dc47c055..bd88bc7484d0 100644 --- a/tests/test_packaging_metadata.py +++ b/tests/test_packaging_metadata.py @@ -1,6 +1,9 @@ from pathlib import Path +import posixpath import re +import runpy import tomllib +import unittest.mock import pytest @@ -15,6 +18,32 @@ REPO_ROOT = Path(__file__).resolve().parents[1] +def _setup_data_files() -> list[tuple[str, list[str]]]: + """Return the ``data_files`` list setup.py passes to ``setuptools.setup()``. + + Runs setup.py via runpy with ``setuptools.setup`` patched to a capture shim + (the same technique as tests/test_docker_webui_install_surface.py), so the + exact wheel data-files the build backend receives are read without running a + real build. setup.py generates these dynamically — see + ``setup.py:bundled_data_files()``. + """ + import setuptools + + captured: dict = {} + + def _capture(**kwargs): + captured.update(kwargs) + + with unittest.mock.patch.object(setuptools, "setup", _capture): + runpy.run_path(str(REPO_ROOT / "setup.py")) + return captured.get("data_files", []) + + +def _shipped_data_sources() -> set[str]: + """All POSIX source paths setup.py ships as wheel data-files.""" + return {src for _, sources in _setup_data_files() for src in sources} + + def _distribution_name(requirement: str) -> str: """Extract the PEP 508 distribution name from a requirement string. @@ -243,17 +272,25 @@ def test_locked_starlette_is_not_vulnerable_to_cve_2026_48710(): def test_locale_catalogs_ship_in_both_wheel_and_sdist(): """Regression test for #27632 / #35374 / #23943. - locales/ is a bare data directory (no __init__.py), so it is invisible to - packages.find and to package-data (which attaches to a package). It must be - declared as setuptools data-files (wheel) AND grafted in MANIFEST.in - (sdist). Without both, sealed installs drop the catalogs and gateway/CLI - commands surface raw i18n keys like `gateway.reset.header_default`. + locales/ is a bare data directory (no __init__.py), so it reaches installs + only as setuptools data-files (wheel) AND a MANIFEST.in graft (sdist). The + wheel data-files are generated in ``setup.py:bundled_data_files()`` — NOT a + ``[tool.setuptools.data-files]`` table in pyproject.toml, which would shadow + setup.py and drop the skills tree (see + ``test_pyproject_does_not_shadow_setup_py_data_files``). Without both + channels, sealed installs drop the catalogs and gateway/CLI commands surface + raw i18n keys like ``gateway.reset.header_default``. """ - data = tomllib.loads((REPO_ROOT / "pyproject.toml").read_text(encoding="utf-8")) - data_files = data["tool"]["setuptools"].get("data-files", {}) - assert data_files.get("locales") == ["locales/*.yaml"], ( - "pyproject [tool.setuptools.data-files] must declare " - 'locales = ["locales/*.yaml"] so the wheel ships i18n catalogs' + shipped = _shipped_data_sources() + on_disk = { + p.relative_to(REPO_ROOT).as_posix() + for p in (REPO_ROOT / "locales").glob("*.yaml") + } + assert on_disk, "expected locales/*.yaml catalogs on disk" + missing = sorted(on_disk - shipped) + assert not missing, ( + f"locale catalogs dropped from wheel data-files: {missing} — " + "setup.py:bundled_data_files() must ship every locales/*.yaml" ) manifest = (REPO_ROOT / "MANIFEST.in").read_text(encoding="utf-8") @@ -261,7 +298,96 @@ def test_locale_catalogs_ship_in_both_wheel_and_sdist(): "MANIFEST.in must `graft locales` so the sdist ships i18n catalogs" ) - # Every on-disk catalog has the .yaml extension the globs above match. - on_disk = list((REPO_ROOT / "locales").glob("*.yaml")) - assert on_disk, "expected locales/*.yaml catalogs on disk" + +def test_bundled_skills_ship_in_wheel_data_files(): + """Regression test: the wheel must ship bundled skills/ and optional-skills/. + + Both are bare data dirs (no __init__.py), so they reach the wheel only via + setuptools ``data_files``. setup.py generates that list dynamically, but a + ``[tool.setuptools.data-files]`` table in pyproject.toml silently OVERRIDES + setup.py's ``data_files`` — which shipped a wheel with ZERO bundled skills + for two releases. On a sealed/wheel install ``_get_bundled_dir()`` then + resolves an absent dir and ``sync_skills()`` returns ``total_bundled: 0``, + so nothing is seeded into ~/.hermes/skills/. + + This drives setup.py's own generator and asserts: + - every on-disk SKILL.md is shipped (completeness), + - the nested category/skill layout is preserved (data_files flattens each + glob into one target dir, so a collapsed tree would corrupt seeding), + - runtime cruft (index-cache, __pycache__, *.pyc) is not shipped. + """ + entries = _setup_data_files() + + shipped: set[str] = set() + for target, sources in entries: + for src in sources: + assert posixpath.dirname(src) == target, ( + f"data_files flatten hazard: {src!r} is not directly under its " + f"declared target {target!r}; setuptools would collapse the tree" + ) + shipped.add(src) + + for root_name in ("skills", "optional-skills"): + on_disk = { + p.relative_to(REPO_ROOT).as_posix() + for p in (REPO_ROOT / root_name).rglob("SKILL.md") + } + assert on_disk, f"expected SKILL.md files under {root_name}/ on disk" + missing = sorted(on_disk - shipped) + assert not missing, ( + f"{root_name}/ skills dropped from wheel data-files: {missing}" + ) + + cruft = sorted( + s + for s in shipped + if "index-cache" in s.split("/") + or "__pycache__" in s.split("/") + or s.endswith((".pyc", ".pyo")) + ) + assert not cruft, f"runtime cruft leaked into wheel data-files: {cruft}" + + +def test_optional_mcps_manifests_ship_in_wheel(): + """Every optional-mcps//manifest.yaml must ship in the wheel and sdist. + + The catalog reader (hermes_cli/mcp_catalog.py) lists whatever manifests are + present in the packaged optional-mcps dir, so a dropped entry silently + removes a server from ``hermes mcp catalog``. Generating data-files from the + tree (vs a hand-maintained pyproject list) keeps this from drifting — the + earlier static list shipped linear + n8n but missed unreal-engine. + """ + shipped = _shipped_data_sources() + on_disk = { + p.relative_to(REPO_ROOT).as_posix() + for p in (REPO_ROOT / "optional-mcps").rglob("manifest.yaml") + } + assert on_disk, "expected optional-mcps//manifest.yaml on disk" + missing = sorted(on_disk - shipped) + assert not missing, ( + f"optional-mcps manifests dropped from wheel data-files: {missing}" + ) + + manifest = (REPO_ROOT / "MANIFEST.in").read_text(encoding="utf-8") + assert "graft optional-mcps" in manifest, ( + "MANIFEST.in must `graft optional-mcps` so the sdist ships the catalog" + ) + + +def test_pyproject_does_not_shadow_setup_py_data_files(): + """pyproject.toml must NOT declare ``[tool.setuptools.data-files]``. + + Bundled data dirs are generated dynamically in + ``setup.py:bundled_data_files()`` because skills/ and optional-skills/ are + too deep to enumerate statically and data_files flattens nested globs. + setuptools resolves ``data_files`` from EITHER pyproject OR setup.py, never + both — a pyproject table wins and silently drops setup.py's list, which + shipped a wheel with zero bundled skills. Keep the table out of pyproject so + setup.py stays the single source of truth. + """ + data = tomllib.loads((REPO_ROOT / "pyproject.toml").read_text(encoding="utf-8")) + assert "data-files" not in data["tool"]["setuptools"], ( + "Remove [tool.setuptools.data-files] from pyproject.toml — it shadows " + "setup.py:bundled_data_files() and drops skills/ from the wheel." + )