Skip to content

fix: handle bytes in skills bundle content hash - #10212

Closed
xiew4589-lang wants to merge 1 commit into
NousResearch:mainfrom
xiew4589-lang:fix/skills-check-bytes-hash
Closed

fix: handle bytes in skills bundle content hash#10212
xiew4589-lang wants to merge 1 commit into
NousResearch:mainfrom
xiew4589-lang:fix/skills-check-bytes-hash

Conversation

@xiew4589-lang

Copy link
Copy Markdown

Summary

  • fix bundle_content_hash() to handle both bytes and str file contents
  • prevent hermes skills check from crashing when a skill bundle contains binary assets
  • add a regression test covering mixed text + binary bundle contents

Verification

  • python -m pytest tests/tools/test_skills_hub.py -q -o 'addopts='
  • hermes skills check
  • python -m pytest tests/tools/ -q -o 'addopts=' shows 5 existing unrelated failures
  • confirmed the same 5 failures also reproduce on origin/main

Existing unrelated test failures on base branch

  • tests/tools/test_file_staleness.py::TestStalenessCheck::test_warning_when_file_modified_externally
  • tests/tools/test_file_staleness.py::TestPatchStaleness::test_patch_warns_on_stale_file
  • tests/tools/test_send_message_missing_platforms.py::TestSendMatrix::test_success
  • tests/tools/test_transcription.py::TestGetProvider::test_explicit_local_no_cloud_fallback
  • tests/tools/test_transcription.py::TestGetProvider::test_local_nothing_available

Root cause

bundle_content_hash() assumed every bundle entry was text and called .encode("utf-8") unconditionally. When a skill bundle included binary content already represented as bytes, hermes skills check crashed with:

AttributeError: 'bytes' object has no attribute 'encode'

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the contribution, @xiew4589-lang!

Closing this as a duplicate of #2740 (by @Mibayy), which targets the same fix/feature. We're consolidating on that PR for review.

If you want to help push it over the line, please jump in there — or if you think your approach is better for a specific reason that isn't covered in the other PR, let us know and we can reopen.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants