Skip to content

fix(skills): handle bytes content in bundle_content_hash - #19157

Closed
venyon2k wants to merge 1 commit into
NousResearch:mainfrom
venyon2k:fix/skills-hub-bytes-hash
Closed

fix(skills): handle bytes content in bundle_content_hash#19157
venyon2k wants to merge 1 commit into
NousResearch:mainfrom
venyon2k:fix/skills-hub-bytes-hash

Conversation

@venyon2k

@venyon2k venyon2k commented May 3, 2026

Copy link
Copy Markdown

Problem

bundle_content_hash() calls .encode("utf-8") on every file entry in SkillBundle.files, but the type annotation explicitly declares files: Dict[str, Union[str, bytes]].

When a SkillBundle is constructed by LocalSource.fetch() — which reads skill files from disk via f.read_bytes() — the files dict contains bytes values. Calling .encode() on a bytes object raises AttributeError.

Fix

Check isinstance(content, bytes) before encoding. Bytes content is fed directly into the SHA-256 hasher; string content is UTF-8 encoded as before.

Evidence

  • SkillBundle annotation: files: Dict[str, Union[str, bytes]] (line 81)
  • LocalSource.fetch() reads files as bytes: files[rel_path] = f.read_bytes() (line 2396 in current main)
  • content_hash() in tools/skills_guard.py already handles both paths — it calls f.read_bytes() directly, the in-memory equivalent should match

Verification

  • Existing test test_bundle_content_hash_matches_installed_content_hash passes (it uses string-only bundles)
  • A test with a binary file in the bundle would demonstrate the crash

Co-authored-by: Jason Kuo jason@yechiu.com.cn

SkillBundle.files can contain bytes (e.g. from LocalSource.fetch()
which reads files via f.read_bytes()), not just strings.
Calling .encode('utf-8') on a bytes value raises AttributeError.

Co-authored-by: Hermes Agent <jason@yechiu.com.cn>
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have tool/skills Skills system (list, view, manage) labels May 3, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Likely duplicate of #19120 (and #19081, #19079) — same fix for bundle_content_hash() calling .encode() on bytes values in SkillBundle.files.

1 similar comment
@alt-glitch

Copy link
Copy Markdown
Collaborator

Likely duplicate of #19120 (and #19081, #19079) — same fix for bundle_content_hash() calling .encode() on bytes values in SkillBundle.files.

@teknium1

teknium1 commented May 4, 2026

Copy link
Copy Markdown
Contributor

The same bytes fix landed via #19575 (salvage of @teknium1's #19328, which was itself the canonical consolidation of five concurrent PRs — including yours). The logic you proposed is identical; thanks for catching and fixing this bug. Both contributors who implemented the same fix get credit via the commit's AUTHOR_MAP and this acknowledgement. Closing as duplicate.

@teknium1 teknium1 closed this May 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low — cosmetic, nice to have 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