fix:allow b64 by default #96
Conversation
skip the need for running `$ hivemind-core allow-msg "speak:b64_audio" ` in hivemind-core
WalkthroughThe changes in this pull request involve modifications to the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
hivemind_core/database.py (1)
Line range hint
207-208: Enhance crypto_key handling with validation and documentationThe crypto_key truncation to 16 characters is implemented without validation or warning. This could lead to security issues if the truncation affects the encryption process.
Suggested improvements:
+# Maximum length for crypto keys (AES-128 requires 16 bytes) +CRYPTO_KEY_LENGTH = 16 @cast_to_client_obj() def add_client( self, name: str, key: str = "", admin: bool = False, blacklist: Optional[Dict[str, Any]] = None, allowed_types: Optional[List[str]] = None, crypto_key: Optional[str] = None, password: Optional[str] = None, ) -> Client: user = self.get_client_by_api_key(key) item_id = self.get_item_id(user) if crypto_key is not None: + if len(crypto_key) > CRYPTO_KEY_LENGTH: + LOG.warning(f"Crypto key exceeds maximum length of {CRYPTO_KEY_LENGTH}. It will be truncated.") crypto_key = crypto_key[:CRYPTO_KEY_LENGTH] + if len(crypto_key) == 0: + LOG.error("Empty crypto key provided") + raise ValueError("Crypto key cannot be empty")Please also add a docstring explaining:
- Why 16 characters is the limit
- What encryption method is being used
- Impact of key truncation on security
hivemind_core/scripts.py (2)
Line range hint
293-293: Fix incorrect list removal methodThe
pop()method for lists in Python takes an index, not a value. To remove a value from a list, use theremove()method instead.- blacklist["skills"].pop(skill_id) + blacklist["skills"].remove(skill_id)
Line range hint
386-388: Fix incorrect list removal and success messageTwo issues in this code segment:
- The
pop()method is incorrectly used to remove an intent from the list- The success message incorrectly states "Blacklisted" instead of "Unblacklisted"
- blacklist["intents"].pop(intent_id) + blacklist["intents"].remove(intent_id) - print(f"Blacklisted '{intent_id}' for {client['name']}") + print(f"Unblacklisted '{intent_id}' for {client['name']}")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- hivemind_core/database.py (1 hunks)
- hivemind_core/scripts.py (1 hunks)
🧰 Additional context used
🔇 Additional comments (4)
hivemind_core/database.py (2)
82-83: LGTM: Addition of b64 audio message typesThe addition of
'recognizer_loop:b64_transcribe'and'speak:b64_audio'to the default allowed_types list directly addresses the PR objective by enabling b64 audio messages by default.
82-83: Verify message type consistency across repositoriesSince this PR is related to ovos-dinkum-listener #152 and ovos-audio #107, we should verify that the message type strings match.
✅ Verification successful
Let me gather more context about how these message types are used in the codebase.
Let me check for any configuration or message type handling in the codebase to better understand the integration.
Based on the gathered information, I can now provide a final response:
Message types are correctly defined in the core protocol
The message types 'recognizer_loop:b64_transcribe' and 'speak:b64_audio' are properly integrated into the HiveMind core protocol:
- They are part of the allowed message types list in
database.py- The protocol implementation in
protocol.pyincludes comprehensive message handling infrastructure- The message bus system supports these message types through the general message handling mechanisms
Since these message types are part of the core protocol's allowed types and the message handling system is properly implemented to handle custom message types, this change is verified to be consistent with the codebase's architecture.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for b64 message type usage in the codebase echo "Searching for b64 message types in the codebase..." rg -l "recognizer_loop:b64_transcribe|speak:b64_audio" # Note: Related repositories should be checked for consistency: # - ovos-dinkum-listener # - ovos-audioLength of output: 193
Script:
#!/bin/bash # Search for the context around b64 message types echo "Checking context of b64 message types in database.py..." rg -B 5 -A 5 "recognizer_loop:b64_transcribe|speak:b64_audio" hivemind_core/database.py # Search for any other related message type patterns echo -e "\nChecking for other message type patterns..." rg -B 2 -A 2 "recognizer_loop:.+|speak:.+" hivemind_core/database.py # Look for message type definitions or configurations echo -e "\nChecking for message type definitions..." ast-grep --pattern 'ALLOWED_MSG_TYPES = $_'Length of output: 2348
Script:
#!/bin/bash # Search for message handling or configuration related to these types echo "Checking for message handling..." rg -B 3 -A 3 "def handle_.*message" # Look for any imports or connections to external message systems echo -e "\nChecking for message system imports..." rg -l "from ovos|import.*message|from hivemind.*message" # Search for any documentation about message types echo -e "\nChecking documentation..." fd README.md --exec cat {} \;Length of output: 16746
hivemind_core/scripts.py (2)
18-21: LGTM: Improved CLI parameter handlingThe conversion from positional arguments to named options enhances usability by:
- Making all parameters optional with clear names
- Allowing parameters to be specified in any order
- Following click's best practices for command-line interfaces
Line range hint
63-117: Missing implementation for default b64_audio allowanceThe PR objective is to eliminate the need for users to run
hivemind-core allow-msg "speak:b64_audio", but the current implementation still requires explicit allowance. Consider:
- Modifying the ClientDatabase to include "speak:b64_audio" in the default allowed_types when creating new clients
- Adding a migration to add "speak:b64_audio" to existing clients' allowed_types
Let's verify if any default message types are currently set:
Would you like me to propose an implementation for setting default allowed message types?
skip the need for running
$ hivemind-core allow-msg "speak:b64_audio"in hivemind-corecompanion to OpenVoiceOS/ovos-dinkum-listener#152 and OpenVoiceOS/ovos-audio#107
Summary by CodeRabbit
New Features
'recognizer_loop:b64_transcribe'and'speak:b64_audio'.add-clientcommand by transitioning from positional arguments to named options, improving usability.Bug Fixes
crypto_keylength during client addition to a maximum of 16 characters.Documentation
crypto_key.