Skip to content

fix(skills): hash bytes bundle contents - #19116

Closed
helix4u wants to merge 1 commit into
NousResearch:mainfrom
helix4u:codex/fix-skills-bundle-hash
Closed

fix(skills): hash bytes bundle contents#19116
helix4u wants to merge 1 commit into
NousResearch:mainfrom
helix4u:codex/fix-skills-bundle-hash

Conversation

@helix4u

@helix4u helix4u commented May 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes the skills hub update/check crash when fetched skill bundle file contents are bytes instead of strings.

SkillBundle.files is typed to allow str | bytes, and some source adapters intentionally populate bytes for bundled assets. bundle_content_hash() only handled strings, so update checks could crash with AttributeError: bytes object has no attribute encode.

Changes

  • Hash bytes bundle contents directly.
  • Continue encoding string bundle contents as UTF-8.
  • Add a regression test for bytes-backed bundle files.

Validation

  • PASS: scripts/run_tests.sh tests/tools/test_skills_hub.py::TestCheckForSkillUpdates::test_bundle_content_hash_accepts_bytes_file_content tests/tools/test_skills_hub.py::TestCheckForSkillUpdates::test_bundle_content_hash_matches_installed_content_hash
  • PASS: scripts/run_tests.sh tests/tools/test_skills_hub.py — 104 passed
  • FAIL: scripts/run_tests.sh — 31 failed, 19090 passed, 51 skipped, 222 warnings in 503.36s. The failures are outside this PR surface; no failures were in tests/tools/test_skills_hub.py.

Full-suite failing tests observed:

  • tests/acp/test_server.py::TestSessionOps::test_send_available_commands_update
  • tests/cli/test_cli_secret_capture.py::test_secret_capture_callback_can_be_completed_from_cli_state_machine
  • tests/gateway/test_approve_deny_commands.py::TestBlockingApprovalE2E::test_blocking_approval_approve_once
  • tests/gateway/test_approve_deny_commands.py::TestBlockingApprovalE2E::test_blocking_approval_deny
  • tests/gateway/test_config.py::TestLoadGatewayConfig::test_bridges_quoted_false_platform_enabled_from_config_yaml
  • tests/agent/test_auxiliary_client.py::TestGetTextAuxiliaryClient::test_custom_endpoint_uses_codex_wrapper_when_runtime_requests_responses_api
  • tests/gateway/test_dingtalk.py::TestCardLifecycle::test_final_reply_finalizes_card
  • tests/gateway/test_dingtalk.py::TestCardLifecycle::test_intermediate_send_stays_streaming
  • tests/gateway/test_dingtalk.py::TestCardLifecycle::test_done_fires_only_when_reply_to_is_set
  • tests/gateway/test_dingtalk.py::TestCardLifecycle::test_edit_message_finalize_fires_done
  • tests/gateway/test_dingtalk.py::TestCardLifecycle::test_edit_message_finalize_false_tracks_sibling
  • tests/gateway/test_dingtalk.py::TestCardLifecycle::test_next_send_auto_closes_sibling_streaming_cards
  • tests/gateway/test_dingtalk.py::TestDingTalkAdapterAICards::test_send_uses_ai_card_if_configured
  • tests/gateway/test_discord_bot_filter.py::TestDiscordBotFilter::test_default_is_none
  • tests/gateway/test_teams.py::TestTeamsSend::test_send_typing
  • tests/gateway/test_api_server.py::TestAdapterInit::test_default_config
  • tests/hermes_cli/test_backup.py::TestProfileRestoration::test_import_creates_profile_wrappers
  • tests/hermes_cli/test_cmd_update.py::TestCmdUpdateBranchFallback::test_update_refreshes_repo_and_tui_node_dependencies
  • tests/hermes_cli/test_update_gateway_restart.py::TestCmdUpdateLaunchdRestart::test_update_restarts_profile_manual_gateways
  • tests/hermes_cli/test_update_gateway_restart.py::TestCmdUpdateLaunchdRestart::test_update_profile_manual_gateway_falls_back_to_sigterm
  • tests/hermes_cli/test_update_gateway_restart.py::TestServicePidExclusion::test_update_kills_manual_pid_but_not_service_pid
  • tests/plugins/test_kanban_dashboard_plugin.py::test_ws_events_rejects_when_token_required
  • tests/run_agent/test_concurrent_interrupt.py::test_concurrent_interrupt_cancels_pending
  • tests/run_agent/test_concurrent_interrupt.py::test_running_concurrent_worker_sees_is_interrupted
  • tests/test_tui_gateway_server.py::test_session_create_drops_pending_title_on_valueerror
  • tests/tools/test_dockerfile_pid1_reaping.py::test_dockerfile_installs_tui_dependencies
  • tests/tools/test_dockerfile_pid1_reaping.py::test_dockerfile_materializes_local_tui_ink_package
  • tests/tools/test_credential_pool_env_fallback.py::TestCredentialPoolSeedsFromDotEnv::test_os_environ_still_wins_over_dotenv
  • tests/tools/test_approval_plugin_hooks.py::TestGatewayPathFiresHooks::test_pre_and_post_fire_on_gateway_surface
  • tests/tools/test_mcp_tool.py::TestSamplingErrors::test_timeout_error
  • tests/tools/test_tirith_security.py::TestDiskFailureMarker::test_cosign_missing_marker_clears_when_cosign_appears

Fixes #13408

@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 #9925 — same root cause in bundle_content_hash() calling .encode('utf-8') on bytes content. Multiple prior fix PRs exist (#9925, #14108, #13531) for #13408.

1 similar comment
@alt-glitch

Copy link
Copy Markdown
Collaborator

Likely duplicate of #9925 — same root cause in bundle_content_hash() calling .encode('utf-8') on bytes content. Multiple prior fix PRs exist (#9925, #14108, #13531) for #13408.

@helix4u

helix4u commented May 3, 2026

Copy link
Copy Markdown
Contributor Author

well fun. my search didn't find any of those. i'll just close it.
new instruction to try and mitigate:
When checking whether a GitHub PR already exists for an issue, do not rely only on GitHub issue “Development” metadata, search results, or gh pr list --search. Always inspect the issue body/comments/timeline for referenced #12345 numbers and open each candidate PR directly with gh pr view <number> --repo <owner>/<repo>. If a search command fails or returns empty, treat that as inconclusive, not proof that no PR exists. Before creating a new PR for a known issue, run a direct PR search by issue number, symptom terms, function/file names, and any referenced PR numbers, then state which candidates were checked.

@helix4u helix4u closed this May 3, 2026
@helix4u
helix4u deleted the codex/fix-skills-bundle-hash branch May 3, 2026 07:34
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.

fix: bundle_content_hash crashes with bytes values (AttributeError)

2 participants