From 9628955dced47d646343ebe7841584831e3fa186 Mon Sep 17 00:00:00 2001 From: kc1awv Date: Fri, 9 Jan 2026 09:54:07 -0500 Subject: [PATCH] fix JOINED/PARTED messages, enhance code quality with type checks and new dependencies, update changelog for version 0.2.1 --- CHANGELOG.md | 6 ++++++ pyproject.toml | 12 ++++++++++++ rrcd/cli.py | 5 ++++- rrcd/commands.py | 14 +++++++++----- rrcd/messages.py | 2 +- rrcd/router.py | 36 ++++++++++++++++++------------------ rrcd/service.py | 8 ++++---- rrcd/session.py | 38 ++++++++++++++++++++++++++++++++++++++ rrcd/stats.py | 8 ++++---- tests/test_resource.py | 23 ++++++++++++----------- 10 files changed, 108 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 83be106..1e71d29 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,12 @@ This project follows the versioning policy in VERSIONING.md. - Joining/parting users continue to receive the full member list (when `include_joined_member_list` is enabled) - See EX1-RRCD.md for detailed protocol documentation +Minor fixes: + +- fixed JOINED/PARTED notification logic to ensure correct member list updates +- improved type checking and annotations in several modules +- added black, ruff, and mypy to development dependencies for code quality enforcement + ## 0.2.0 - 2026-01-07 - **Major internal refactoring**: Improved code organization and maintainability diff --git a/pyproject.toml b/pyproject.toml index 6245938..501728d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -35,7 +35,9 @@ Issues = "https://github.com/kc1awv/rrcd/issues" [project.optional-dependencies] dev = [ "pytest>=8.0.0", + "black>=24.0.0", "ruff>=0.6.0", + "mypy>=1.0.0", ] [project.scripts] @@ -54,3 +56,13 @@ line-length = 100 [tool.ruff.lint] select = ["E", "F", "I", "B", "UP"] ignore = ["E501"] + +[tool.mypy] +python_version = "3.11" +warn_return_any = true +warn_unused_configs = true +disallow_untyped_defs = false + +[[tool.mypy.overrides]] +module = "RNS.*" +ignore_missing_imports = true diff --git a/rrcd/cli.py b/rrcd/cli.py index 3dd4726..a6c5945 100644 --- a/rrcd/cli.py +++ b/rrcd/cli.py @@ -386,8 +386,11 @@ def main(argv: list[str] | None = None) -> None: # Use ConfigManager to load config file if config_path: from .config import ConfigManager + # Create temporary manager for loading - temp_hub = type('obj', (object,), {'config': cfg, 'log': None, '_state_lock': None})() + temp_hub = type( + "obj", (object,), {"config": cfg, "log": None, "_state_lock": None} + )() temp_mgr = ConfigManager(temp_hub) # type: ignore data = temp_mgr.load_toml(config_path) cfg = temp_mgr.apply_config_data(cfg, data) diff --git a/rrcd/commands.py b/rrcd/commands.py index 6b689ea..9c3c1fb 100644 --- a/rrcd/commands.py +++ b/rrcd/commands.py @@ -123,8 +123,8 @@ class CommandHandler: ) return True - st = self.hub.room_manager._room_state_get(r) - if st and st.get("private"): + room_state = self.hub.room_manager._room_state_get(r) + if room_state is not None and room_state.get("private"): if not self.hub.trust_manager.is_server_op(peer_hash): self.hub.message_helper.emit_notice( outgoing, link, None, f"room {r} is private" @@ -882,7 +882,11 @@ class CommandHandler: for other in list(self.hub.room_manager.get_room_members(r)): s = self.hub.session_manager.sessions.get(other) ph = s.get("peer") if s else None - if isinstance(ph, (bytes, bytearray)) and bytes(ph) == target_hash: + if ( + isinstance(ph, (bytes, bytearray)) + and bytes(ph) == target_hash + and s is not None + ): s.get("rooms", set()).discard(r) self.hub.room_manager.get_room_members(r).discard(other) if self.hub.identity is not None: @@ -1022,8 +1026,8 @@ class CommandHandler: return True target_hash = bytes(ph) - key = st.get("key") - is_keyed = isinstance(key, str) and bool(key) + key_val = st.get("key") + is_keyed = isinstance(key_val, str) and bool(key_val) is_invite_only = bool(st.get("invite_only", False)) if is_keyed: diff --git a/rrcd/messages.py b/rrcd/messages.py index c73d77e..bef5bbf 100644 --- a/rrcd/messages.py +++ b/rrcd/messages.py @@ -37,7 +37,7 @@ class MessageHelper: """Check if payload fits within link MDU without creating/packing packets.""" try: if hasattr(link, "MDU") and link.MDU is not None: - return len(payload) <= link.MDU + return bool(len(payload) <= link.MDU) pkt = RNS.Packet(link, payload) pkt.pack() return True diff --git a/rrcd/router.py b/rrcd/router.py index 4f857d5..a9caae3 100644 --- a/rrcd/router.py +++ b/rrcd/router.py @@ -492,16 +492,17 @@ class MessageRouter: self.hub.room_manager.touch_room(r) - # Notify existing room members about the new joiner - # Send JOINED message with single identity hash to existing members existing_members = [ member_link for member_link in self.hub.room_manager.get_room_members(r) if member_link != link ] - if existing_members: + if existing_members and self.hub.identity is not None: + notification_body = ( + [peer_hash] if self.hub.config.include_joined_member_list else None + ) member_notification = make_envelope( - T_JOINED, src=self.hub.identity.hash, room=r, body=[peer_hash] + T_JOINED, src=self.hub.identity.hash, room=r, body=notification_body ) member_notification_payload = encode(member_notification) for member_link in existing_members: @@ -509,7 +510,6 @@ class MessageRouter: outgoing, member_link, member_notification_payload ) - # Send JOINED message with full member list to the joining user joined_body = None if self.hub.config.include_joined_member_list: members: list[bytes] = [] @@ -520,10 +520,11 @@ class MessageRouter: members.append(bytes(ph)) joined_body = members - joined = make_envelope( - T_JOINED, src=self.hub.identity.hash, room=r, body=joined_body - ) - self.hub.message_helper.queue_env(outgoing, link, joined) + if self.hub.identity is not None: + joined = make_envelope( + T_JOINED, src=self.hub.identity.hash, room=r, body=joined_body + ) + self.hub.message_helper.queue_env(outgoing, link, joined) try: inv = st.get("invited") @@ -581,14 +582,13 @@ class MessageRouter: return sess["rooms"].discard(r) - - # Get remaining members before removing the parting user + remaining_members = [ member_link for member_link in self.hub.room_manager.get_room_members(r) if member_link != link ] - + if self.hub.room_manager.get_room_members(r): self.hub.room_manager.remove_member(r, link) if not self.hub.room_manager.get_room_members(r): @@ -601,11 +601,12 @@ class MessageRouter: if st is not None and not st.get("registered"): self.hub.room_manager._room_state.pop(r, None) - # Notify remaining members about the user parting - # Send PARTED message with single identity hash to remaining members - if remaining_members: + if remaining_members and self.hub.identity is not None: + notification_body = ( + [peer_hash] if self.hub.config.include_joined_member_list else None + ) member_notification = make_envelope( - T_PARTED, src=self.hub.identity.hash, room=r, body=[peer_hash] + T_PARTED, src=self.hub.identity.hash, room=r, body=notification_body ) member_notification_payload = encode(member_notification) for member_link in remaining_members: @@ -613,7 +614,6 @@ class MessageRouter: outgoing, member_link, member_notification_payload ) - # Send PARTED message with current member list to the parting user parted_body = None if self.hub.config.include_joined_member_list: members: list[bytes] = [] @@ -697,7 +697,7 @@ class MessageRouter: return try: - r = self.hub._norm_room(room) + r = self.hub._norm_room(str(room)) if room else "" except Exception as e: if self.hub.identity is not None: self.hub.message_helper.emit_error( diff --git a/rrcd/service.py b/rrcd/service.py index 9d92835..316e072 100644 --- a/rrcd/service.py +++ b/rrcd/service.py @@ -60,8 +60,8 @@ class HubService: self.identity = self._load_identity(self.config.identity_path) self.trust_manager.load_from_config( - self.config.trusted_identities, - self.config.banned_identities, + list(self.config.trusted_identities), + list(self.config.banned_identities), ) self._load_registered_rooms_from_registry() @@ -363,7 +363,7 @@ class HubService: """Resolve token to identity hash. Returns hash if successful, None otherwise. For ambiguous matches, use _resolve_identity_hash_with_matches instead. """ - target_link = self._find_target_link(token, room=room) + target_link = self.command_handler._find_target_link(token, room=room) if target_link is not None: s = self.session_manager.sessions.get(target_link) ph = s.get("peer") if s else None @@ -381,7 +381,7 @@ class HubService: Returns (hash, matches) tuple. Hash is None if ambiguous or not found. Use matches list to provide helpful error messages. """ - matches = self._find_target_links(token, room=room) + matches = self.command_handler._find_target_links(token, room=room) if len(matches) == 1: s = self.session_manager.sessions.get(matches[0]) diff --git a/rrcd/session.py b/rrcd/session.py index c2f1302..0878c7f 100644 --- a/rrcd/session.py +++ b/rrcd/session.py @@ -104,6 +104,10 @@ class SessionManager: (peer_hash, nick, rooms_count) for logging Must be called with state lock held. """ + from .codec import encode + from .constants import T_PARTED + from .envelope import make_envelope + sess = self.sessions.pop(link, None) self._rate.pop(link, None) @@ -120,9 +124,43 @@ class SessionManager: if nick: self.update_nick_index(link, nick, None) + # Notify remaining members and remove from rooms + peer_hash = bytes(peer) if isinstance(peer, (bytes, bytearray)) else None for room in list(sess["rooms"]): + # Get remaining members before removing the disconnecting user + remaining_members = [ + member_link + for member_link in self.hub.room_manager.get_room_members(room) + if member_link != link + ] + + # Remove the disconnecting user from the room self.hub.room_manager.remove_member(room, link) + # Send PARTED notification to remaining members + if remaining_members and peer_hash and self.hub.identity: + notification_body = ( + [peer_hash] if self.hub.config.include_joined_member_list else None + ) + member_notification = make_envelope( + T_PARTED, + src=self.hub.identity.hash, + room=room, + body=notification_body, + ) + member_notification_payload = encode(member_notification) + for member_link in remaining_members: + try: + import RNS + + RNS.Packet(member_link, member_notification_payload).send() + self.hub.stats_manager.inc( + "bytes_out", len(member_notification_payload) + ) + except Exception: + # Link may already be closed or in bad state, ignore send failures + pass + return peer, nick, rooms_count def update_nick_index( diff --git a/rrcd/stats.py b/rrcd/stats.py index 0c01743..56fcb0e 100644 --- a/rrcd/stats.py +++ b/rrcd/stats.py @@ -12,7 +12,7 @@ if TYPE_CHECKING: class StatsManager: """ Manages hub statistics collection and reporting. - + Tracks counters for: - Bytes in/out - Packets processed @@ -28,10 +28,10 @@ class StatsManager: def __init__(self, hub: HubService) -> None: self.hub = hub self.log = hub.log - + self.started_wall_time: float | None = None self.started_monotonic: float | None = None - + self._counters: dict[str, int] = { "bytes_in": 0, "bytes_out": 0, @@ -71,7 +71,7 @@ class StatsManager: def format_stats(self) -> str: """Format current statistics as a human-readable string.""" from . import __version__ - + now_mono = time.monotonic() started_mono = self.started_monotonic uptime_s = (now_mono - started_mono) if started_mono is not None else 0.0 diff --git a/tests/test_resource.py b/tests/test_resource.py index 838d439..1ad8fd5 100644 --- a/tests/test_resource.py +++ b/tests/test_resource.py @@ -1,4 +1,5 @@ """Tests for resource transfer functionality.""" + import hashlib import os @@ -24,7 +25,7 @@ def test_resource_envelope_serialization(): rid = os.urandom(8) payload = b"This is a test payload that is larger than typical MDU" sha256 = hashlib.sha256(payload).digest() - + body = { B_RES_ID: rid, B_RES_KIND: RES_KIND_NOTICE, @@ -32,21 +33,21 @@ def test_resource_envelope_serialization(): B_RES_SHA256: sha256, B_RES_ENCODING: "utf-8", } - + envelope = make_envelope( T_RESOURCE_ENVELOPE, src=src, room="test", body=body, ) - + # Serialize and deserialize encoded = encode(envelope) decoded = decode(encoded) - + assert decoded[K_T] == T_RESOURCE_ENVELOPE assert decoded[K_SRC] == src - + decoded_body = decoded[K_BODY] assert decoded_body[B_RES_ID] == rid assert decoded_body[B_RES_KIND] == RES_KIND_NOTICE @@ -59,22 +60,22 @@ def test_resource_envelope_minimal(): """Test resource envelope with minimal required fields.""" src = os.urandom(16) rid = os.urandom(8) - + body = { B_RES_ID: rid, B_RES_KIND: "blob", B_RES_SIZE: 1024, } - + envelope = make_envelope( T_RESOURCE_ENVELOPE, src=src, body=body, ) - + encoded = encode(envelope) decoded = decode(encoded) - + decoded_body = decoded[K_BODY] assert B_RES_SHA256 not in decoded_body assert B_RES_ENCODING not in decoded_body @@ -85,12 +86,12 @@ def test_sha256_verification(): """Test SHA256 hash computation for payload verification.""" payload = b"Test payload for SHA256 verification" expected = hashlib.sha256(payload).digest() - + # Verify we can compute and compare hashes correctly computed = hashlib.sha256(payload).digest() assert computed == expected assert len(computed) == 32 - + # Verify mismatch detection wrong_payload = b"Different payload" wrong_hash = hashlib.sha256(wrong_payload).digest()