Skip to content

fix(packaging): ship bundled skills in the wheel via setup.py data_files - #52113

Open
Que0x wants to merge 2 commits into
NousResearch:mainfrom
Que0x:fix/ship-bundled-skills-wheel
Open

fix(packaging): ship bundled skills in the wheel via setup.py data_files#52113
Que0x wants to merge 2 commits into
NousResearch:mainfrom
Que0x:fix/ship-bundled-skills-wheel

Conversation

@Que0x

@Que0x Que0x commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Problem

Wheel installs shipped zero bundled skills. setup.py generates data_files for skills/ + optional-skills/, but pyproject's [tool.setuptools.data-files] table silently overrides it — setuptools resolves that field from one source, never both. So _get_bundled_dir() finds nothing and sync_skills() returns total_bundled: 0 on every wheel/sealed install. The earlier 3a7ed7be0 ("ship bundled skills in wheel") never actually took effect.

Fix

Make setup.py the single source of truth: dynamically generate data_files for all four bundled dirs (skills, optional-skills, locales, optional-mcps) and remove the shadowing pyproject table. Side fixes in the generator: emit POSIX path separators (was baking in backslash targets), exclude index-cache/__pycache__/*.pyc cruft, and auto-include the silently-dropped optional-mcps/unreal-engine.

Tests

  • Wheel rebuilt + inspected: skills/ 447 files / 72 SKILL.md, optional-skills/ 444 / 101, locales/ 16, optional-mcps/ 3 — nested structure preserved, no cruft (before: skills/optional-skills = 0). sdist unchanged (still ships skills via MANIFEST).
  • pytest tests/test_packaging_metadata.py11 passed (rewrote the locale test for the new source; added 3 regressions: skills completeness/no-flatten/no-cruft, optional-mcps drift guard, pyproject-shadow guard).
  • ruff check setup.py tests/test_packaging_metadata.pyclean.

pyproject's [tool.setuptools.data-files] table silently overrode setup.py's
data_files, so wheels shipped zero skills/ and optional-skills/ and sync_skills()
returned total_bundled: 0 on sealed installs. Consolidate all bundled data dirs
into one dynamic generator in setup.py (also fixes a Windows path-separator bug,
excludes index-cache/__pycache__ cruft, and auto-includes the silently-dropped
optional-mcps/unreal-engine). Remove the shadowing pyproject table and add
regression tests.
@Que0x
Que0x requested a review from a team June 24, 2026 19:39
@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard tool/skills Skills system (list, view, manage) P2 Medium — degraded but workaround exists labels Jun 24, 2026
runpy.run_path runs setup.py as "<run_path>", not "__main__", so the guard
added earlier in this branch stopped setuptools.setup() from running and broke
test_docker_webui_install_surface. Remove the guard; the packaging tests now
capture data_files via runpy + a setuptools.setup patch (the same technique the
docker-webui test uses) instead of importing the module.
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for tracing this to the competing setuptools configuration sources. The premise still holds on current main: setup.py:83-86 supplies skill trees through data_files, while pyproject.toml:315-336 declares a separate [tool.setuptools.data-files] table. The packaged-install resolution path consumes <sysconfig data>/skills before source fallbacks (hermes_constants.py:206-223), and sync_skills() returns total_bundled: 0 when that directory is absent (tools/skills_sync.py:505-511).

The PR's single dynamic generator preserves per-directory targets, includes the existing locales and MCP catalog payloads, and its proposed tests cover source completeness, target layout, excluded artifacts, and reintroduction of the shadowing table. No verified correctness issue was found in the diff. The affected packaging sections have not materially changed on current main, so salvage should be mechanical.

Automated hermes-sweeper review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/install-update Installer, updater, packaging, wheels, doctor comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows tool/skills Skills system (list, view, manage) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants