Skip to content

feat(gateway): support bundled .skill documents - #9615

Open
LevSky22 wants to merge 2 commits into
NousResearch:mainfrom
LevSky22:feat/skill-document-support
Open

feat(gateway): support bundled .skill documents#9615
LevSky22 wants to merge 2 commits into
NousResearch:mainfrom
LevSky22:feat/skill-document-support

Conversation

@LevSky22

@LevSky22 LevSky22 commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds content-aware .skill document support across the main gateway adapters.

Some .skill files are plaintext markdown-like skills, while others are ZIP bundles containing SKILL.md, references, and assets. This PR admits .skill uploads across the shared gateway allowlist, then classifies each payload by bytes instead of assuming every .skill file is text.

That means:

  • plaintext .skill payloads continue to inject inline like existing text documents
  • ZIP-backed .skill payloads are cached as archives instead of being incorrectly injected as markdown text

This is the same general class of gateway-document enhancement as #4105 / #4109, but for a Hermes-specific skill packaging use case.

Latest upstream adaptation

Rebased onto upstream main at 8081425a1c095d01db858ea1a574d17c93703f48 and resolved the Slack conflict by preserving upstream's expanded text-snippet injection while adding content-aware .skill classification.

Related Issue

Related to:

Type of Change

  • ✨ New feature (non-breaking change that adds functionality)
  • ✅ Tests (adding or improving test coverage)

Changes Made

  • gateway/platforms/base.py
    • changed .skill from a hardcoded text MIME to content-aware classification
    • added classify_document_mime() to distinguish ZIP-backed vs plaintext .skill payloads
  • gateway/platforms/telegram.py
    • classify .skill uploads by bytes before deciding whether to inject text
  • gateway/platforms/slack.py
    • same content-aware .skill handling for downloaded files
  • gateway/platforms/discord.py
    • same content-aware .skill handling for downloaded attachments
  • gateway/platforms/feishu.py
    • skip text injection when a cached .skill file is actually a ZIP archive
  • gateway/platforms/whatsapp.py
    • skip text injection when a cached .skill file is actually a ZIP archive
  • tests:
    • tests/gateway/test_document_cache.py
    • tests/gateway/test_telegram_documents.py
    • tests/gateway/test_slack.py
    • tests/gateway/test_discord_document_handling.py

How to Test

  1. Upload a plaintext .skill file through Telegram, Slack, or Discord
  2. Confirm the file is accepted and its content is injected into the event text like .md / .txt
  3. Upload a ZIP-backed .skill bundle
  4. Confirm the file is accepted, cached as an archive, and not injected as bogus markdown text

Automated validation performed after rebasing:

python -m pytest -o addopts="" \
  tests/gateway/test_document_cache.py \
  tests/gateway/test_telegram_documents.py \
  tests/gateway/test_slack.py \
  tests/gateway/test_discord_document_handling.py -q -ra

Result:

  • 230 passed, 27 warnings

The warning set was existing asyncio / Slack AsyncMock noise, not introduced by this PR.

Manual validation performed:

  • live-tested on a running Docker deployment with a real .skill upload that was actually a ZIP bundle
  • Hermes detected the PK header and extracted the bundle instead of treating it as inline markdown text

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: Ubuntu 24.04 / Docker

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Screenshots / Logs

Plaintext .skill:

  • accepted and injected as text

ZIP-backed .skill:

  • accepted as an archive
  • detected via PK header
  • extracted by the runtime instead of being injected as text

@RuckVibeCodes RuckVibeCodes 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.

[gus-first-pass] feat(gateway): support bundled .skill documents - Clear feature improvement, no issues found.

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/gateway Gateway runner, session dispatch, delivery tool/skills Skills system (list, view, manage) labels Apr 27, 2026
@LevSky22
LevSky22 force-pushed the feat/skill-document-support branch from 6e83990 to 28afc88 Compare April 28, 2026 05:26
@LevSky22

Copy link
Copy Markdown
Contributor Author

Updated this PR for the latest upstream main.

  • Rebased the branch onto 8081425a1c095d01db858ea1a574d17c93703f48.
  • Resolved the Slack conflict by preserving upstream's expanded text-snippet injection and layering the .skill MIME classification on top.
  • Reviewed the final diff after rebase; I did not find additional code-review issues in the adapted patch.
  • Targeted gateway validation passed: 230 passed, 27 warnings for tests/gateway/test_document_cache.py, tests/gateway/test_telegram_documents.py, tests/gateway/test_slack.py, and tests/gateway/test_discord_document_handling.py.

The warnings are the existing asyncio / Slack AsyncMock warnings already noted in the PR, not new failures from this change.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the content-aware .skill handling work. The underlying gap still exists on current main, but this branch no longer reaches the running adapters.

Problems

  • gh api repos/NousResearch/hermes-agent/pulls/9615 reports mergeable_state: dirty. The five edited gateway/platforms/*.py adapter files were removed by 5600105478ffde29d7566b45421b100eaa29c4ef; current adapters live in plugins/platforms/.
  • Current main still has no .skill entry in gateway/platforms/base.py:1333-1383. The active Telegram, Slack, and Discord paths select text injection through that shared policy at plugins/platforms/telegram/adapter.py:7977-8002, plugins/platforms/slack/adapter.py:3102-3129, and plugins/platforms/discord/adapter.py:6406-6449; Feishu and WhatsApp have distinct extraction paths at plugins/platforms/feishu/adapter.py:3827-3841 and plugins/platforms/whatsapp/adapter.py:1457-1481.

Suggested changes

  • Port the byte-aware .skill classification and injection gate to the five current plugin adapters, then move the tests to those live paths. Add plaintext and ZIP-backed coverage for Feishu and WhatsApp too.

Automated hermes-sweeper review.

Comment thread gateway/platforms/base.py
@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 12, 2026
@LevSky22
LevSky22 force-pushed the feat/skill-document-support branch from 28afc88 to 9ad8883 Compare July 13, 2026 07:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have 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-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages tool/skills Skills system (list, view, manage) type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants