Skip to content

fix(build): ship built-in skills in wheel by consolidating data_files in setup.py - #66768

Closed
kyssta-exe wants to merge 1 commit into
NousResearch:mainfrom
kyssta-exe:auto-fix/issue-66733
Closed

kyssta-exe wants to merge 1 commit into
NousResearch:mainfrom
kyssta-exe:auto-fix/issue-66733

Conversation

@kyssta-exe

Copy link
Copy Markdown
Contributor

Problem

Issue #66733: Built-in skills are missing from the wheel. This is a regression caused by commit c349eca, which added a [tool.setuptools.data-files] section to pyproject.toml. According to PEP 621 / setuptools documentation, when [tool.setuptools.data-files] is present in pyproject.toml, it overrides (does not merge with) the data_files argument in setup.py. Since the pyproject.toml section only declared locales and optional-mcps, the skills/ and optional-skills/ entries from setup.py were silently dropped from the wheel.

Fix

Remove the [tool.setuptools.data-files] section from pyproject.toml and consolidate all data-file declarations into setup.py using the existing _data_file_tree() helper:

  • `_data_file_tree('skills') — was already there, now works again
  • `_data_file_tree('locales') — moved from pyproject.toml

The _data_file_tree() helper preserves the subdirectory structure (it groups files by parent directory), avoiding the flattening problem that would occur with a simple skills/**/* glob in the TOML format.

Verification

  • Wheel built with python -m build --wheel: all four data directories (skills, optional-skills, locales, optional-mcps) are present in hermes_agent-0.18.2.data/data/
  • Subdirectory structure preserved: skills/category/skill-name/SKILL.md hierarchy intact
  • Packaging tests pass: test_manifest_includes_bundled_skills, test_locale_catalogs_covered_by_data_file_tree

Closes #66733

@kyssta-exe
kyssta-exe requested a review from a team July 18, 2026 06:11
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard tool/skills Skills system (list, view, manage) tool/mcp MCP client and OAuth labels Jul 18, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Related to #66733 and the broader open fix #52113. Both remove the shadowing data-files table, but #52113 also hardens generated tree filtering and packaging coverage; this narrower repair is competing work, not a duplicate.

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed by Hermes Agent

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed by Hermes Agent

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for tracing the competing setuptools declarations. The premise is confirmed on current main: setup.py:83-86 supplies the skill trees, while pyproject.toml:315-336 defines a separate data-files table.

Problems

  • Deleting that table reactivates setup.py:_data_file_tree() for the skills payload, but setup.py:67-75 includes every regular file recursively. The current tree contains skills/index-cache/*.json and skills/.hub metadata; nix/hermes-agent.nix:62-65 explicitly filters cache directories from its bundled-skills source. The wheel path needs equivalent filtering before this is enabled.
  • The added locale check reads setup.py and matches a literal helper call. AGENTS.md:1380-1433 bans source-text tests; this would pass even if the helper generated the wrong wheel payload. Existing tests/test_wheel_locales_e2e.py:38-91 shows the appropriate built-wheel, isolated-install pattern.

Suggested changes

  • Filter runtime/cache artifacts in _data_file_tree() while preserving actual skill payload files.
  • Replace the source-text assertion with generator or installed-wheel behavioral coverage, including a bundled-skills assertion.

Automated hermes-sweeper review.

@teknium1 teknium1 added 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 sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform area/install-update Installer, updater, packaging, wheels, doctor labels Jul 18, 2026
@kyssta-exe

Copy link
Copy Markdown
Contributor Author

This PR has merge conflicts with the base branch and cannot be merged in its current state. Closing as part of automated PR maintenance — please rebase and re-open if the fix is still needed.

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-moderate Sweeper blast radius: moderate — a subsystem or single platform 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/mcp MCP client and OAuth 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.

[Bug]: Built-in skills missing from wheel

4 participants