Skip to content

fix(packaging): ship bundled skills in wheel by unifying data-files in setup.py (#66733) - #68310

Open
JonthanaHanh wants to merge 1 commit into
NousResearch:mainfrom
JonthanaHanh:fix/skills-missing-from-wheel
Open

fix(packaging): ship bundled skills in wheel by unifying data-files in setup.py (#66733)#68310
JonthanaHanh wants to merge 1 commit into
NousResearch:mainfrom
JonthanaHanh:fix/skills-missing-from-wheel

Conversation

@JonthanaHanh

Copy link
Copy Markdown
Contributor

Summary

[tool.setuptools.data-files] in pyproject.toml overrides setup.py's data_files when both are present. Since the pyproject section only declared locales and optional-mcps, the skills/ and optional-skills/ directories were silently dropped from the wheel.

This means pip install hermes-agent ships zero bundled skills — hermes_constants._get_packaged_data_dir("skills") returns None, and tools/skills_sync.py has nothing to sync to ~/.hermes/skills/.

Fix

Move locales and optional-mcps into setup.py's data_files (via the existing _data_file_tree() helper), then remove the [tool.setuptools.data-files] section entirely. setup.py is now the single source of truth for all four bare data directories:

  • skills/
  • optional-skills/
  • locales/
  • optional-mcps/

This preserves the same installation semantics — _data_file_tree() produces identical (directory, [files]) tuples for all four trees — while eliminating the override that caused the regression.

Verification

$ python3 -m pytest tests/test_packaging_metadata.py -x
11 passed

Updated test_locale_catalogs_ship_in_both_wheel_and_sdist to check setup.py instead of the removed pyproject.toml section.

Fixes #66733

@JonthanaHanh
JonthanaHanh requested a review from a team July 21, 2026 01:17
@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) tool/mcp MCP client and OAuth P2 Medium — degraded but workaround exists sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades area/install-update Installer, updater, packaging, wheels, doctor duplicate This issue or pull request already exists labels Jul 21, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #66768: the live patch makes the same setuptools data-files consolidation for skills, locales, and optional MCP manifests. Please consolidate with the earlier focused PR.

…n setup.py

pyproject.toml's [tool.setuptools.data-files] section overrides
setup.py's data_files when both are present. Since the pyproject
section only declared locales and optional-mcps, the skills and
optional-skills directories were silently dropped from the wheel.

This meant pip-installed hermes-agent had zero bundled skills —
hermes_constants._get_packaged_data_dir('skills') returned None,
and tools/skills_sync.py had nothing to sync to ~/.hermes/skills/.

Fix: move locales and optional-mcps into setup.py's data_files
(via _data_file_tree), then remove the [tool.setuptools.data-files]
section entirely. setup.py is now the single source of truth for all
four bare data directories.

Fixes NousResearch#66733
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for tracing the original setuptools shadowing issue. Current main has moved away from the wheel-distribution path this PR repairs.

Problems

  • setup.py:27-74 is now only a wheel/sdist build guard; it has no _data_file_tree() or data_files payload for this patch to extend. Commit d84e11af4d removed the former packaging path and packaging metadata tests.
  • website/docs/getting-started/platform-support.md:40-47 now marks PyPI installs unsupported and states that PRs fixing unsupported distribution methods will not be accepted.

Suggested changes

  • This is not mechanically salvageable onto current main; retain it for maintainer disposition rather than attempting the prior wheel-payload consolidation.

Automated hermes-sweeper review.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 30, 2026
@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Summary

Nine PRs address or reference this issue complex. #23738/#28421 established wheel payload discovery, #23884/#28368 seed an already-present payload for Kanban, and #52113/#66768/#66846/#67058/#68310 remove the setuptools configuration conflict; however, the latest current-main review records that the relevant wheel data-files path has since been removed and PyPI installs are unsupported.

Related pull requests

Duplicates

#23738/#28421 are the original and merged packaging-precursor pair; #23884/#28368 are the corresponding Kanban-seeding pair. #66768, #66846, #67058, and #68310 share the core four-tree consolidation implemented more completely by #52113.

Suggested consolidation

Keep #52113 open with a salvage path conditional on maintainers restoring a supported wheel-distribution target, then adapt its filtered generator and payload-level guards to the replacement mechanism; this preserves its recorded best-fix status and agrees with its keep_open review without treating the stale historical path as actionable. Despite the duplicate relationship, keep #68310 open with that same conditional salvage path for maintainer disposition because its newer keep_open review explicitly records no mechanical salvage on current main; #66768, #66846, and #67058 are already closed competing implementations, while #28368 and #28421 remain merged historical references.

Complex graph

flowchart TD
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I23725(["issue #23725 (closed)"])
    I66733(["issue #66733 (open)"])
    subgraph Dup52113 ["PRs duplicating each other"]
        P52113["PR #52113 (open)"]
        P66768["PR #66768 (closed)"]
        P66846["PR #66846 (closed)"]
        P67058["PR #67058 (closed)"]
        P68310["PR #68310 (open)"]
    end
    P68310 -->|fixes| I23725
    P68310 -->|fixes| I66733
    class I23725 closed
    class I66733 open
    class P52113 open
    class P66768 closed
    class P66846 closed
    class P67058 closed
    class P68310 open
    class P52113 best
    class P68310 target
    click I23725 "https://github.com/NousResearch/hermes-agent/issues/23725"
    click I66733 "https://github.com/NousResearch/hermes-agent/issues/66733"
    click P52113 "https://github.com/NousResearch/hermes-agent/pull/52113"
    click P66768 "https://github.com/NousResearch/hermes-agent/pull/66768"
    click P66846 "https://github.com/NousResearch/hermes-agent/pull/66846"
    click P67058 "https://github.com/NousResearch/hermes-agent/pull/67058"
    click P68310 "https://github.com/NousResearch/hermes-agent/pull/68310"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 9 pull requests and 2 issues in this complex. Each diff was read against this issue; Assessment working set: 66 kB of PR diffs, 13 kB of issue/PR text, 8 kB of discussion (19 comments), 26 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

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 duplicate This issue or pull request already exists 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 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