From 6a3510ce2eec9ef5db8904835fa297947fec2fca Mon Sep 17 00:00:00 2001 From: Jack Kingsman Date: Fri, 27 Feb 2026 15:17:29 -0800 Subject: [PATCH] Misc. doc, test, and qol improvements --- AGENTS.md | 5 +- app/AGENTS.md | 2 +- app/radio_sync.py | 13 +- app/routers/contacts.py | 4 +- docker-compose.yaml | 3 + frontend/package.json | 2 +- frontend/src/components/AGENTS.md | 2 +- frontend/src/hooks/index.ts | 1 + .../src/test/useWebSocket.dispatch.test.ts | 26 ++++ tests/test_packet_pipeline.py | 130 ++++++++++++++++++ tests/test_radio_operation.py | 53 ++++++- tests/test_radio_sync.py | 58 +++++++- 12 files changed, 277 insertions(+), 22 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 246af97..07b13c5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -150,8 +150,10 @@ This message-layer echo/path handling is independent of raw-packet storage dedup . ├── app/ # FastAPI backend │ ├── AGENTS.md # Backend documentation +│ ├── bot.py # Bot execution and outbound bot sends │ ├── main.py # App entry, lifespan │ ├── routers/ # API endpoints +│ ├── packet_processor.py # Raw packet pipeline, dedup, path handling │ ├── repository.py # Database CRUD │ ├── event_handlers.py # Radio events │ ├── decoder.py # Packet decryption @@ -167,7 +169,6 @@ This message-layer echo/path handling is independent of raw-packet storage dedup │ │ ├── MapView.tsx # Leaflet map showing node locations │ │ └── ... │ └── vite.config.ts -├── references/meshcore_py/ # MeshCore Python library ├── tests/ # Backend tests (pytest) ├── data/ # SQLite database (runtime) └── pyproject.toml # Python dependencies @@ -389,6 +390,8 @@ mc.subscribe(EventType.ACK, handler) | `MESHCORE_TCP_PORT` | `4000` | TCP port (used with `MESHCORE_TCP_HOST`) | | `MESHCORE_BLE_ADDRESS` | *(none)* | BLE device address (mutually exclusive with serial/TCP) | | `MESHCORE_BLE_PIN` | *(required with BLE)* | BLE PIN code | +| `MESHCORE_SERIAL_BAUDRATE` | `115200` | Serial baud rate | +| `MESHCORE_LOG_LEVEL` | `INFO` | Logging level (`DEBUG`/`INFO`/`WARNING`/`ERROR`) | | `MESHCORE_DATABASE_PATH` | `data/meshcore.db` | SQLite database location | **Note:** Runtime app settings are stored in the database (`app_settings` table), not environment variables. These include `max_radio_contacts`, `auto_decrypt_dm_on_advert`, `sidebar_sort_order`, `advert_interval`, `last_advert_time`, `favorites`, `last_message_times`, and `bots`. They are configured via `GET/PATCH /api/settings` (and related settings endpoints). diff --git a/app/AGENTS.md b/app/AGENTS.md index c76066b..e79bb2b 100644 --- a/app/AGENTS.md +++ b/app/AGENTS.md @@ -8,7 +8,7 @@ Keep it aligned with `app/` source files and router behavior. - FastAPI - aiosqlite - Pydantic -- MeshCore Python library (`references/meshcore_py`) +- MeshCore Python library (`meshcore` from PyPI) - PyCryptodome ## Backend Map diff --git a/app/radio_sync.py b/app/radio_sync.py index d543bae..db756f2 100644 --- a/app/radio_sync.py +++ b/app/radio_sync.py @@ -443,6 +443,8 @@ async def _periodic_advert_loop(): """ while True: try: + await asyncio.sleep(ADVERT_CHECK_INTERVAL) + # Try to send - send_advertisement() handles all checks # (disabled, throttled, not connected) if radio_manager.is_connected: @@ -455,9 +457,6 @@ async def _periodic_advert_loop(): except RadioOperationBusyError: logger.debug("Skipping periodic advertisement: radio busy") - # Sleep before next check - await asyncio.sleep(ADVERT_CHECK_INTERVAL) - except asyncio.CancelledError: logger.info("Periodic advertisement task cancelled") break @@ -595,9 +594,7 @@ async def _sync_contacts_to_radio_inner(mc: MeshCore) -> dict: break if len(selected_contacts) < max_contacts: - recent_contacts = await ContactRepository.get_recent_non_repeaters( - limit=max_contacts - ) + recent_contacts = await ContactRepository.get_recent_non_repeaters(limit=max_contacts) for contact in recent_contacts: key = contact.public_key.lower() if key in selected_keys: @@ -658,9 +655,7 @@ async def _sync_contacts_to_radio_inner(mc: MeshCore) -> dict: } -async def sync_recent_contacts_to_radio( - force: bool = False, mc: MeshCore | None = None -) -> dict: +async def sync_recent_contacts_to_radio(force: bool = False, mc: MeshCore | None = None) -> dict: """ Load contacts to the radio for DM ACK support. diff --git a/app/routers/contacts.py b/app/routers/contacts.py index 1eb9627..ccfc55d 100644 --- a/app/routers/contacts.py +++ b/app/routers/contacts.py @@ -539,7 +539,9 @@ async def create_contact( "last_contacted": existing.last_contacted, } ) - existing.name = request.name + refreshed = await ContactRepository.get_by_key(request.public_key) + if refreshed is not None: + existing = refreshed # Trigger historical decryption if requested (even for existing contacts) if request.try_historical: diff --git a/docker-compose.yaml b/docker-compose.yaml index 230e30a..10fdc03 100644 --- a/docker-compose.yaml +++ b/docker-compose.yaml @@ -26,4 +26,7 @@ services: # TCP # MESHCORE_TCP_HOST: 192.168.1.100 # MESHCORE_TCP_PORT: 4000 + + # Logging + # MESHCORE_LOG_LEVEL: INFO restart: unless-stopped diff --git a/frontend/package.json b/frontend/package.json index f07dd1a..f02fb01 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -24,7 +24,6 @@ "@radix-ui/react-separator": "^1.1.8", "@radix-ui/react-slot": "^1.2.4", "@radix-ui/react-tabs": "^1.1.13", - "@types/three": "^0.182.0", "@uiw/react-codemirror": "^4.25.4", "class-variance-authority": "^0.7.1", "clsx": "^2.1.1", @@ -48,6 +47,7 @@ "@testing-library/react": "^16.0.0", "@types/d3-force": "^3.0.10", "@types/leaflet": "^1.9.21", + "@types/three": "^0.182.0", "@types/node": "^25.0.3", "@types/react": "^18.3.12", "@types/react-dom": "^18.3.1", diff --git a/frontend/src/components/AGENTS.md b/frontend/src/components/AGENTS.md index ddb9390..1f09167 100644 --- a/frontend/src/components/AGENTS.md +++ b/frontend/src/components/AGENTS.md @@ -52,7 +52,7 @@ Types, constants, and pure functions shared across the codebase: - Types: `NodeType`, `PacketLabel`, `Particle`, `ObservedPath`, `PendingPacket`, `ParsedPacket`, `TrafficObservation`, `RepeaterTrafficData`, `RepeaterSplitAnalysis` - Constants: `COLORS`, `PARTICLE_COLOR_MAP`, `PARTICLE_SPEED`, `DEFAULT_OBSERVATION_WINDOW_SEC`, traffic thresholds, `PACKET_LEGEND_ITEMS` -- Functions: `simpleHash`, `parsePacket`, `getPacketLabel`, `generatePacketKey`, `getLinkId`, `getNodeType`, `dedupeConsecutive`, `analyzeRepeaterTraffic`, `recordTrafficObservation` +- Functions: `hashString` (from `utils/contactAvatar.ts`), `parsePacket`, `getPacketLabel`, `generatePacketKey`, `getLinkId`, `getNodeType`, `dedupeConsecutive`, `analyzeRepeaterTraffic`, `recordTrafficObservation` `GraphNode` and `GraphLink` are defined locally in the component — they extend `SimulationNodeDatum3D` and `SimulationLinkDatum` from `d3-force-3d`. diff --git a/frontend/src/hooks/index.ts b/frontend/src/hooks/index.ts index 7ff0db7..62acf24 100644 --- a/frontend/src/hooks/index.ts +++ b/frontend/src/hooks/index.ts @@ -1,6 +1,7 @@ export { useUnreadCounts } from './useUnreadCounts'; export { useConversationMessages, getMessageContentKey } from './useConversationMessages'; export { useRadioControl } from './useRadioControl'; +export { useRepeaterDashboard } from './useRepeaterDashboard'; export { useAppSettings } from './useAppSettings'; export { useConversationRouter } from './useConversationRouter'; export { useContactsAndChannels } from './useContactsAndChannels'; diff --git a/frontend/src/test/useWebSocket.dispatch.test.ts b/frontend/src/test/useWebSocket.dispatch.test.ts index 88d2208..169f295 100644 --- a/frontend/src/test/useWebSocket.dispatch.test.ts +++ b/frontend/src/test/useWebSocket.dispatch.test.ts @@ -177,6 +177,32 @@ describe('useWebSocket dispatch', () => { Object.values(handlers).forEach((fn) => expect(fn).not.toHaveBeenCalled()); }); + it('routes raw_packet event to onRawPacket with observation_id', () => { + const onRawPacket = vi.fn(); + renderHook(() => useWebSocket({ onRawPacket })); + + const rawPacketData = { + id: 5, + observation_id: 42, + timestamp: 1700000000, + data: 'aabbccdd', + payload_type: 'GROUP_TEXT', + snr: 7.5, + rssi: -85, + decrypted: true, + decrypted_info: { + channel_name: '#general', + sender: 'Alice', + }, + }; + fireMessage({ type: 'raw_packet', data: rawPacketData }); + + expect(onRawPacket).toHaveBeenCalledOnce(); + expect(onRawPacket).toHaveBeenCalledWith(rawPacketData); + expect(onRawPacket.mock.calls[0][0]).toHaveProperty('observation_id', 42); + expect(onRawPacket.mock.calls[0][0]).toHaveProperty('id', 5); + }); + it('malformed JSON calls no handlers (catch branch)', () => { const handlers = { onHealth: vi.fn(), diff --git a/tests/test_packet_pipeline.py b/tests/test_packet_pipeline.py index bccea18..80b784c 100644 --- a/tests/test_packet_pipeline.py +++ b/tests/test_packet_pipeline.py @@ -2075,3 +2075,133 @@ class TestRunHistoricalDmDecryption: assert len(success_calls) == 1 assert "2 messages" in success_calls[0]["details"] + + +class TestHistoricalDMDirectionDetection: + """Test direction detection in run_historical_dm_decryption. + + Verifies the BUG-2 fix: when first public key bytes of our key and the + contact's key collide (1/256 chance), the function must default to + outgoing=False rather than mis-classifying the message. + """ + + # Our key: first byte is 0xAA + OUR_PUB_HEX = "AA" + "00" * 31 + OUR_PUB = bytes.fromhex(OUR_PUB_HEX) + OUR_PRIV = b"\x01" * 64 # Dummy, won't be used (try_decrypt_dm is mocked) + + # Contact key: first byte differs (0xBB) — normal case + CONTACT_DIFF_PUB_HEX = "bb" + "11" * 31 + CONTACT_DIFF_PUB = bytes.fromhex(CONTACT_DIFF_PUB_HEX) + + # Contact key: first byte same as ours (0xAA) — the 1/256 collision case + CONTACT_SAME_PUB_HEX = "aa" + "22" * 31 + CONTACT_SAME_PUB = bytes.fromhex(CONTACT_SAME_PUB_HEX) + + def _make_text_message_bytes(self, unique_suffix: bytes = b"") -> bytes: + """Build a minimal raw packet with TEXT_MESSAGE payload type.""" + header = 0x09 # route_type=FLOOD(0x01), payload_type=TEXT_MESSAGE(0x02) + path_length = 0 + payload = bytes([0xAA, 0xBB]) + b"\x00\x00" + b"\xab" * 16 + unique_suffix + return bytes([header, path_length]) + payload + + @pytest.mark.asyncio + async def test_incoming_dm_marked_as_incoming(self, test_db, captured_broadcasts): + """Normal case: src_hash differs from our first byte -> outgoing=False (incoming).""" + from app.packet_processor import run_historical_dm_decryption + + raw = self._make_text_message_bytes(b"\x60") + await RawPacketRepository.create(raw, 7000) + + mock_packet_info = PacketInfo( + route_type=1, + payload_type=PayloadType.TEXT_MESSAGE, + payload_version=0, + path_length=0, + path=b"", + payload=b"\xaa\xbb" + b"\x00" * 18, + ) + + # src_hash="bb" (contact), dest_hash="aa" (us) -> incoming + mock_decrypted = DecryptedDirectMessage( + timestamp=7000, + flags=0, + message="Hello from contact", + dest_hash="aa", + src_hash="bb", + ) + + broadcasts, mock_broadcast = captured_broadcasts + + with patch("app.packet_processor.broadcast_event", mock_broadcast): + with patch("app.packet_processor.try_decrypt_dm", return_value=mock_decrypted): + with patch("app.packet_processor.parse_packet", return_value=mock_packet_info): + with patch("app.packet_processor.derive_public_key", return_value=self.OUR_PUB): + with patch( + "app.packet_processor.create_dm_message_from_decrypted", + new_callable=AsyncMock, + return_value=100, + ) as mock_create: + with patch("app.websocket.broadcast_success"): + await run_historical_dm_decryption( + self.OUR_PRIV, + self.CONTACT_DIFF_PUB, + self.CONTACT_DIFF_PUB_HEX, + ) + + mock_create.assert_awaited_once() + call_kwargs = mock_create.call_args[1] + assert call_kwargs["outgoing"] is False + + @pytest.mark.asyncio + async def test_ambiguous_first_bytes_defaults_to_incoming(self, test_db, captured_broadcasts): + """1/256 case: our_public_key_bytes[0] == contact_public_key_bytes[0]. + + Both src_hash and dest_hash match our first byte. The function must + default to outgoing=False (incoming) because outgoing DMs are stored + by the send endpoint, so historical decryption only recovers incoming. + """ + from app.packet_processor import run_historical_dm_decryption + + raw = self._make_text_message_bytes(b"\x61") + await RawPacketRepository.create(raw, 7100) + + mock_packet_info = PacketInfo( + route_type=1, + payload_type=PayloadType.TEXT_MESSAGE, + payload_version=0, + path_length=0, + path=b"", + payload=b"\xaa\xaa" + b"\x00" * 18, + ) + + # Both hashes are "aa" — matches our first byte (0xAA) + mock_decrypted = DecryptedDirectMessage( + timestamp=7100, + flags=0, + message="Ambiguous direction msg", + dest_hash="aa", + src_hash="aa", + ) + + broadcasts, mock_broadcast = captured_broadcasts + + with patch("app.packet_processor.broadcast_event", mock_broadcast): + with patch("app.packet_processor.try_decrypt_dm", return_value=mock_decrypted): + with patch("app.packet_processor.parse_packet", return_value=mock_packet_info): + with patch("app.packet_processor.derive_public_key", return_value=self.OUR_PUB): + with patch( + "app.packet_processor.create_dm_message_from_decrypted", + new_callable=AsyncMock, + return_value=101, + ) as mock_create: + with patch("app.websocket.broadcast_success"): + await run_historical_dm_decryption( + self.OUR_PRIV, + self.CONTACT_SAME_PUB, + self.CONTACT_SAME_PUB_HEX, + ) + + mock_create.assert_awaited_once() + call_kwargs = mock_create.call_args[1] + assert call_kwargs["outgoing"] is False diff --git a/tests/test_radio_operation.py b/tests/test_radio_operation.py index 248447d..74be246 100644 --- a/tests/test_radio_operation.py +++ b/tests/test_radio_operation.py @@ -1,7 +1,7 @@ """Tests for shared radio operation locking behavior.""" import asyncio -from unittest.mock import AsyncMock, MagicMock +from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -169,3 +169,54 @@ class TestRadioOperationYield: async with radio_manager.radio_operation("test_swap") as yielded: assert yielded is new_mc assert yielded is not old_mc + + +class TestRequireConnected: + """Test the require_connected() FastAPI dependency.""" + + def test_raises_503_when_setup_in_progress(self): + """HTTPException 503 is raised when radio is connected but setup is still in progress.""" + from fastapi import HTTPException + + from app.dependencies import require_connected + + with patch("app.dependencies.radio_manager") as mock_rm: + mock_rm.is_connected = True + mock_rm.meshcore = MagicMock() + mock_rm.is_setup_in_progress = True + + with pytest.raises(HTTPException) as exc_info: + require_connected() + + assert exc_info.value.status_code == 503 + assert "initializing" in exc_info.value.detail.lower() + + def test_raises_503_when_not_connected(self): + """HTTPException 503 is raised when radio is not connected.""" + from fastapi import HTTPException + + from app.dependencies import require_connected + + with patch("app.dependencies.radio_manager") as mock_rm: + mock_rm.is_setup_in_progress = False + mock_rm.is_connected = False + mock_rm.meshcore = None + + with pytest.raises(HTTPException) as exc_info: + require_connected() + + assert exc_info.value.status_code == 503 + + def test_returns_meshcore_when_connected_and_setup_complete(self): + """Returns meshcore instance when radio is connected and setup is complete.""" + from app.dependencies import require_connected + + mock_mc = MagicMock() + with patch("app.dependencies.radio_manager") as mock_rm: + mock_rm.is_setup_in_progress = False + mock_rm.is_connected = True + mock_rm.meshcore = mock_mc + + result = require_connected() + + assert result is mock_mc diff --git a/tests/test_radio_sync.py b/tests/test_radio_sync.py index 9ac883e..7da25ac 100644 --- a/tests/test_radio_sync.py +++ b/tests/test_radio_sync.py @@ -364,6 +364,48 @@ class TestSyncRecentContactsToRadio: assert result["loaded"] == 0 assert result["failed"] == 1 + @pytest.mark.asyncio + async def test_mc_param_bypasses_lock_acquisition(self, test_db): + """When mc is passed, the function uses it directly without acquiring radio_operation. + + This tests the BUG-1 fix: sync_and_offload_all already holds the lock, + so it passes mc directly to avoid deadlock (asyncio.Lock is not reentrant). + """ + await _insert_contact(KEY_A, "Alice", last_contacted=2000) + + mock_mc = MagicMock() + mock_mc.get_contact_by_key_prefix = MagicMock(return_value=None) + mock_result = MagicMock() + mock_result.type = EventType.OK + mock_mc.commands.add_contact = AsyncMock(return_value=mock_result) + + # Make radio_operation raise if called — it should NOT be called + # when mc is provided + def radio_operation_should_not_be_called(*args, **kwargs): + raise AssertionError("radio_operation should not be called when mc is passed") + + with patch.object( + radio_manager, "radio_operation", side_effect=radio_operation_should_not_be_called + ): + result = await sync_recent_contacts_to_radio(mc=mock_mc) + + assert result["loaded"] == 1 + mock_mc.commands.add_contact.assert_called_once() + + @pytest.mark.asyncio + async def test_mc_param_still_respects_throttle(self): + """When mc is passed but throttle is active (not forced), it should still return throttled.""" + mock_mc = MagicMock() + + # First call to set _last_contact_sync + radio_manager._meshcore = mock_mc + await sync_recent_contacts_to_radio() + + # Second call with mc= but no force — should still be throttled + result = await sync_recent_contacts_to_radio(mc=mock_mc) + assert result["throttled"] is True + assert result["loaded"] == 0 + @pytest.mark.asyncio async def test_uses_post_lock_meshcore_after_swap(self, test_db): """If _meshcore is swapped between pre-check and lock acquisition, @@ -1032,9 +1074,10 @@ class TestPeriodicAdvertLoopRaces: is caught by the outer except — loop survives and continues.""" rm, _mc = _make_connected_manager() _disconnect_on_acquire(rm) - # Advert loop: work first, then sleep. On error the except-handler - # sleeps too, so we need 2 sleeps before cancel. - mock_sleep, sleep_calls = _sleep_controller(cancel_after=2) + # Advert loop: sleep first, then work. Sleep 1 (loop top) passes, + # work hits RadioDisconnectedError, error handler does sleep 2 (passes), + # next iteration sleep 3 cancels cleanly via except CancelledError. + mock_sleep, sleep_calls = _sleep_controller(cancel_after=3) with ( patch("app.radio_sync.radio_manager", rm), @@ -1044,15 +1087,15 @@ class TestPeriodicAdvertLoopRaces: await _periodic_advert_loop() mock_advert.assert_not_called() - assert len(sleep_calls) == 2 + assert len(sleep_calls) == 3 @pytest.mark.asyncio async def test_busy_lock_skips_iteration(self): """RadioOperationBusyError is caught and send_advertisement is not called.""" rm, _mc = _make_connected_manager() lock = await _pre_hold_lock(rm) - # Busy path falls through to normal sleep (only 1 needed) - mock_sleep, _ = _sleep_controller(cancel_after=1) + # Sleep 1 (loop top) passes, work hits busy error, sleep 2 cancels. + mock_sleep, _ = _sleep_controller(cancel_after=2) try: with ( @@ -1071,7 +1114,8 @@ class TestPeriodicAdvertLoopRaces: """The mc yielded by radio_operation() is forwarded to send_advertisement — not a stale radio_manager.meshcore read.""" rm, mock_mc = _make_connected_manager() - mock_sleep, _ = _sleep_controller(cancel_after=1) + # Sleep 1 (loop top) passes through, work runs, sleep 2 cancels. + mock_sleep, _ = _sleep_controller(cancel_after=2) with ( patch("app.radio_sync.radio_manager", rm),