Conversation
… license gates, and split stable/nightly PyPI release lanes (#16) * adds docs site and ci docs build * Stop tracking ancient-docs (reference-only, in .gitignore) * Ignore site/ (MkDocs build output), stop tracking * resolves url , site/ ignore and uv install security * adds dashboard * removes stray md * adds memory system * adds memory system (#6) * adds memory system * fix failing tests in ci * fix failing tests in ci * adds trusted publisher * adds multiband and relay support * adds multiband and relay support (#7) * adds multiband and relay support * Update radioshaq/radioshaq/listener/relay_delivery.py * update tests * adds dbstate __getattr__ * docs ci update pinned mkdocs version * adds radio frequency * adds radio outbound reply default * adds ci test on pr * solves frequency and send tx race conditions * fixes pending entries & mode clobbering (fm) * fixes configuration hardcoded path and fixed reject path misleading error , cleans up pending list api semantics * resolves code review issues * Add frequency-aware radio replies (#8) * adds radio frequency * adds radio outbound reply default * adds ci test on pr * solves frequency and send tx race conditions * fixes pending entries & mode clobbering (fm) * fixes configuration hardcoded path and fixed reject path misleading error , cleans up pending list api semantics * resolves code review issues * adds ci fixes * Update radioshaq/radioshaq/api/routes/messages.py * solves code review suggestions * fixes failing tests * Fixes PyPI UI bundling flow (#12) * adds radio frequency * adds radio outbound reply default * adds ci test on pr * solves frequency and send tx race conditions * fixes pending entries & mode clobbering (fm) * fixes configuration hardcoded path and fixed reject path misleading error , cleans up pending list api semantics * resolves code review issues * adds ci fixes * Update radioshaq/radioshaq/api/routes/messages.py * solves code review suggestions * fixes failing tests * fixes build patch * fixes minor issues * fixes build env bug (#13) * adds radio frequency * adds radio outbound reply default * adds ci test on pr * solves frequency and send tx race conditions * fixes pending entries & mode clobbering (fm) * fixes configuration hardcoded path and fixed reject path misleading error , cleans up pending list api semantics * resolves code review issues * adds ci fixes * Update radioshaq/radioshaq/api/routes/messages.py * solves code review suggestions * fixes failing tests * fixes build patch * fixes minor issues * fixes ci issue * Update radioshaq/tests/integration/test_live_e2e_workflows.py * fixes ci issue (#14) * adds radio frequency * adds radio outbound reply default * adds ci test on pr * solves frequency and send tx race conditions * fixes pending entries & mode clobbering (fm) * fixes configuration hardcoded path and fixed reject path misleading error , cleans up pending list api semantics * resolves code review issues * adds ci fixes * Update radioshaq/radioshaq/api/routes/messages.py * solves code review suggestions * fixes failing tests * fixes build patch * fixes minor issues * fixes ci issue * Update radioshaq/tests/integration/test_live_e2e_workflows.py * Fix settings TS build and include web UI in wheel * adds release , versioning and workflows * adds ci , build , enfore branch protection and more * adds contributing * chore: bump version to 0.1.1 * fix(ci): allow non-interactive GPL acceptance in CLI setup tests * fix(ci): harden publish/governance workflows and refresh action versions * Fix CI branch guards, release push race handling, and license/version checks * adds contributing * adds validation fixes
* adds radio frequency * adds radio outbound reply default * adds ci test on pr * solves frequency and send tx race conditions * fixes pending entries & mode clobbering (fm) * fixes configuration hardcoded path and fixed reject path misleading error , cleans up pending list api semantics * resolves code review issues * adds ci fixes * Update radioshaq/radioshaq/api/routes/messages.py * solves code review suggestions * fixes failing tests * fixes build patch * fixes minor issues * fixes ci issue * Update radioshaq/tests/integration/test_live_e2e_workflows.py * Fix settings TS build and include web UI in wheel * adds release , versioning and workflows * adds ci , build , enfore branch protection and more * adds contributing * chore: bump version to 0.1.1 * fix(ci): allow non-interactive GPL acceptance in CLI setup tests * fix(ci): harden publish/governance workflows and refresh action versions * Fix CI branch guards, release push race handling, and license/version checks * adds contributing * adds validation fixes * adds contributing , license gate , ci build and release , pre push testing * adds docs site and ci docs build * Stop tracking ancient-docs (reference-only, in .gitignore) * Ignore site/ (MkDocs build output), stop tracking * resolves url , site/ ignore and uv install security * adds dashboard * removes stray md * adds memory system * adds memory system (#6) * adds memory system * fix failing tests in ci * fix failing tests in ci * adds trusted publisher * adds multiband and relay support * adds multiband and relay support (#7) * adds multiband and relay support * Update radioshaq/radioshaq/listener/relay_delivery.py * update tests * adds dbstate __getattr__ * docs ci update pinned mkdocs version * adds radio frequency * adds radio outbound reply default * adds ci test on pr * solves frequency and send tx race conditions * fixes pending entries & mode clobbering (fm) * fixes configuration hardcoded path and fixed reject path misleading error , cleans up pending list api semantics * resolves code review issues * Add frequency-aware radio replies (#8) * adds radio frequency * adds radio outbound reply default * adds ci test on pr * solves frequency and send tx race conditions * fixes pending entries & mode clobbering (fm) * fixes configuration hardcoded path and fixed reject path misleading error , cleans up pending list api semantics * resolves code review issues * adds ci fixes * Update radioshaq/radioshaq/api/routes/messages.py * solves code review suggestions * fixes failing tests * Fixes PyPI UI bundling flow (#12) * adds radio frequency * adds radio outbound reply default * adds ci test on pr * solves frequency and send tx race conditions * fixes pending entries & mode clobbering (fm) * fixes configuration hardcoded path and fixed reject path misleading error , cleans up pending list api semantics * resolves code review issues * adds ci fixes * Update radioshaq/radioshaq/api/routes/messages.py * solves code review suggestions * fixes failing tests * fixes build patch * fixes minor issues * fixes build env bug (#13) * adds radio frequency * adds radio outbound reply default * adds ci test on pr * solves frequency and send tx race conditions * fixes pending entries & mode clobbering (fm) * fixes configuration hardcoded path and fixed reject path misleading error , cleans up pending list api semantics * resolves code review issues * adds ci fixes * Update radioshaq/radioshaq/api/routes/messages.py * solves code review suggestions * fixes failing tests * fixes build patch * fixes minor issues * fixes ci issue * Update radioshaq/tests/integration/test_live_e2e_workflows.py * fixes ci issue (#14) * adds radio frequency * adds radio outbound reply default * adds ci test on pr * solves frequency and send tx race conditions * fixes pending entries & mode clobbering (fm) * fixes configuration hardcoded path and fixed reject path misleading error , cleans up pending list api semantics * resolves code review issues * adds ci fixes * Update radioshaq/radioshaq/api/routes/messages.py * solves code review suggestions * fixes failing tests * fixes build patch * fixes minor issues * fixes ci issue * Update radioshaq/tests/integration/test_live_e2e_workflows.py * Fix settings TS build and include web UI in wheel * adds release , versioning and workflows * adds ci , build , enfore branch protection and more * adds contributing * chore: bump version to 0.1.1 * fix(ci): allow non-interactive GPL acceptance in CLI setup tests * fix(ci): harden publish/governance workflows and refresh action versions * Fix CI branch guards, release push race handling, and license/version checks * adds contributing * adds validation fixes * Update GitHub Sponsors username in FUNDING.yml * Update .githooks/pre-push * adds license fallbacks for installed package
* readme improvements and readthedocs ci * remove broken link
|
@greptileai , comprehensively update your review based on the changes above : |
This reverts commit 9ab7010.
|
@greptileai , comprehensively update your review based on the recent changes made , investigate all relevant in diff and out of diff revisions , produce a new flowchart accordingly , produce a comprehensive review and update : |
| """Minimal loguru stub for environments without the real dependency. | ||
|
|
||
| This stub provides a drop-in `logger` object with no-op methods so that | ||
| code importing `from loguru import logger` continues to run in test and | ||
| development environments where loguru is not installed. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from typing import Any | ||
|
|
||
|
|
||
| class _Logger: | ||
| def __getattr__(self, name: str): | ||
| def _noop(*args: Any, **kwargs: Any) -> None: | ||
| return None | ||
|
|
||
| return _noop | ||
|
|
||
| def __call__(self, *args: Any, **kwargs: Any) -> None: # pragma: no cover - trivial | ||
| return None | ||
|
|
||
|
|
||
| logger = _Logger() | ||
|
|
There was a problem hiding this comment.
Stub module shadows the real loguru package, silencing all logging
This file creates a local Python package named loguru at radioshaq/loguru/__init__.py. Because the radioshaq/ top-level directory is typically in sys.path during development, testing (pytest run from radioshaq/), and editable installs (pip install -e . / uv sync), Python's import resolver will find this stub before the installed loguru package. The result is that every from loguru import logger call throughout the entire codebase silently resolves to the no-op _Logger stub, dropping all log output in production.
The file comment says "for environments without the real dependency", but the placement makes it active even when loguru is installed, defeating its stated purpose.
Fix: Guard the stub so it only activates when loguru is genuinely absent, or relocate it out of the top-level directory. For example, wrap it at the call site:
try:
from loguru import logger
except ImportError:
from radioshaq._loguru_stub import loggerOr rename/move the file so it does not shadow the installed package (e.g. radioshaq/radioshaq/_loguru_stub.py).
| "signature_validated": signature_validated, | ||
| } | ||
|
|
||
| # Opt-out handling (STOP): record directly if DB available, then acknowledge. | ||
| if body.upper() in _OPTOUT_KEYWORDS: | ||
| db = getattr(request.app.state, "db", None) | ||
| if db is not None and hasattr(db, "record_opt_out_by_phone"): | ||
| try: |
There was a problem hiding this comment.
STOP opt-out silently discarded on DB failure — regulatory compliance risk
When a STOP message arrives and db.record_opt_out_by_phone raises an exception, the error is swallowed and the user receives "You have been opted out" even though their opt-out was never persisted. Subsequent messages will continue to be sent to that number.
Under TCPA (US), CASL (CA), and similar regulations, silently failing to honour an opt-out while confirming it to the user is a compliance violation.
try:
await db.record_opt_out_by_phone(from_phone, channel)
except Exception as e:
logger.warning("Opt-out record failed ...")
# ← success response sent anyway, opt-out not recorded
return _twiml_response("You have been opted out. Reply START to re-subscribe.")Consider returning a generic error TwiML (or no response, which causes Twilio to retry) when the opt-out cannot be recorded, so the user can try again:
try:
await db.record_opt_out_by_phone(from_phone, channel)
return _twiml_response("You have been opted out. Reply START to re-subscribe.")
except Exception as e:
logger.error("Opt-out record failed (channel={} phone={}): {}", channel, from_phone, e)
# Return 500 so Twilio retries, or at minimum do not confirm success
raise HTTPException(status_code=500, detail="Opt-out could not be recorded")| raise HTTPException(status_code=503, detail="Database not available") | ||
| if not (hasattr(db, "get_coordination_event_by_id_raw") or hasattr(db, "get_coordination_event_by_id")): | ||
| raise HTTPException(status_code=503, detail="Database not available") | ||
| get_event = getattr(db, "get_coordination_event_by_id_raw", db.get_coordination_event_by_id) | ||
| # Atomic claim: only one concurrent approval can transition pending -> approving | ||
| claimed = await db.claim_emergency_event_pending(event_id) |
There was a problem hiding this comment.
approve returns HTTP 200 ok=True with status=pending when approval actually failed
When the message bus is unavailable, the event is correctly rolled back to "pending", but the response is:
{"ok": true, "event_id": ..., "status": "pending", "sent": false, "detail": "Message bus not available"}From the caller's perspective, they invoked POST /emergency/events/{id}/approve and received ok: true — yet the event was not approved and no message was sent. This is misleading and could cause the UI to show "approved" state while the event remains unactioned.
The same pattern exists in the publish_outbound failure branch (lines 193-199) and the queue-full branch (lines 200-203).
Consider returning a non-200 status code (e.g. HTTP 503) when the approval cannot be completed, so callers can distinguish a successful approval from a failed one:
if not message_bus or not hasattr(message_bus, "publish_outbound"):
await db.update_coordination_event(event_id, status="pending")
raise HTTPException(
status_code=503,
detail="Message bus not available; approval rolled back to pending",
)
Greptile Summary
Release 0.1.4 is a large feature release adding emergency coordination (§9) with SSE streams and approve/reject workflows, Twilio SMS/WhatsApp webhooks with opt-out handling, a pluggable compliance plugin system covering FCC/CEPT/AU/CA/JP/IN/NZ and ~30 regional backends, pluggable ASR/TTS plugin registries (Voxtral, Whisper, Scribe, ElevenLabs, Kokoro), a remote SDR broker for HackRF TX, occupied-bandwidth compliance checking, new GIS endpoints, and contact preferences with explicit-consent gating for EU/UK/ZA regions.
Key issues found:
radioshaq/loguru/__init__.pyshadows reallogurupackage: A no-op logger stub placed atradioshaq/loguru/__init__.pywill be resolved before the installedlogurupackage wheneverradioshaq/is insys.path(development, editable installs,pytest), silencing all logging in production and tests.twilio.pyacknowledges the opt-out to the user even when the database call fails, meaning the opt-out is not persisted and future messages will still be sent, violating TCPA/CASL opt-out obligations.approvereturns HTTP 200ok=Truewhen approval rolls back topending: When the message bus is unavailable, the event is rolled back but the response still signalsok: true, making it impossible for callers to distinguish a successful approval from a failed one.shakods/model-ID prefix in ASR plugin: The_is_voxtral_like_model_idfunction still containss.startswith("shakods/"), a leftover from the SHAKODS → RadioShaq rename made in the same PR.COUNT(*):_get_pending_emergency_countfetches up to 1000 full ORM rows every 10 seconds (SSE tick) to count them; aSELECT COUNT(*)query would be far more efficient.Confidence Score: 2/5
radioshaq/loguru/__init__.pyplaced in the top-level project directory shadows the reallogurupackage in every development and CI environment, turning all log calls into no-ops silently; (2) STOP opt-out acknowledgement sent even when DB persistence fails, which is a TCPA/CASL compliance violation. Several other previously-flagged issues (null dereference inget_latest_location_decoded, TOCTOU inupdate_coordination_event, GET handler mutating shared state, TX success flag) remain unaddressed from the previous review cycle. The XSS fixes viaescapeHtmland the atomicclaim_emergency_event_pendingare positive steps.radioshaq/loguru/__init__.py(critical — shadows real loguru),radioshaq/radioshaq/api/routes/twilio.py(opt-out compliance),radioshaq/radioshaq/api/routes/emergency.py(approve status contract),radioshaq/radioshaq/api/routes/config_routes.py(still mutates shared state on GET)Important Files Changed
logurupackage, silencing all production and development logging.approvereturns HTTP 200ok=Trueeven when the message bus is unavailable and the approval is rolled back.shakods/model-ID prefix leftover from the project rename.get_latest_location_decoded,claim_emergency_event_pending(atomic),update_coordination_event,get_pending_coordination_events; pending count uses O(N) full-row fetch instead of COUNT(*), and null geometry dereference inget_latest_location_decodedremains.&,<,>,", and'._ComplianceCheckedTransmitterbase class, occupied-bandwidth compliance check, remote broker TX, andpyhackrf2dependency;_auditnow accepts asuccessflag addressing prior silent-failure concern.X-Config-Effective-Afterheader and_metafield; addshuggingface_api_keyandgemini_api_keyto redacted keys set; GET handler still mutates the shared override dict in-place (previously flagged, not fixed).Sequence Diagram
sequenceDiagram participant UI as Operator UI participant API as FastAPI (emergency.py) participant DB as PostGIS (postgres_gis.py) participant Bus as MessageBus (nanobot) participant Disp as OutboundDispatcher participant Twilio as Twilio SMS/WhatsApp UI->>API: POST /emergency/request API->>API: emergency_messaging_allowed(region, config) API->>API: normalize_e164(contact_phone) API->>DB: store_coordination_event(status=pending) DB-->>API: event_id API-->>UI: {ok: true, event_id, status: pending} Note over UI,API: Operator reviews pending emergencies UI->>API: GET /emergency/events/stream (SSE) loop Every 10s API->>DB: get_pending_coordination_events(max_results=1000) DB-->>API: events list API-->>UI: data: {pending_count: N} end UI->>API: POST /emergency/events/{id}/approve API->>DB: claim_emergency_event_pending(id) — atomic UPDATE WHERE status=pending DB-->>API: event_id (or None if already processed) API->>DB: get_coordination_event_by_id_raw(id) DB-->>API: {extra_data: {phone, channel}} API->>Bus: publish_outbound(OutboundMessage channel=sms|whatsapp) Bus-->>API: ok=True API->>DB: update_coordination_event(status=approved, approved_at/by) API-->>UI: {ok: true, status: approved, queued: true} Note over Bus,Disp: Async consumer loop Disp->>Bus: consume_outbound() Bus-->>Disp: OutboundMessage(channel=sms, phone, content) Disp->>Disp: agent_registry.get_agent("sms") Disp->>Twilio: sms_agent.execute({action: send, to, message}) Twilio-->>Disp: success Disp->>DB: update_coordination_event(sent_at)Comments Outside Diff (4)
radioshaq/radioshaq/config/schema.py, line 171-174 (link)Security: Hardcoded default JWT secret key
This is a well-known anti-pattern. If a deployment fails to override this value, all JWT tokens are signed with a publicly known secret, allowing any attacker to forge authentication tokens. While the comment says "must be secure in production", there is no runtime check or warning.
Consider:
debug=False.radioshaq/radioshaq/config/schema.py, line 126-129 (link)Default postgres URL contains hardcoded credentials
The default includes username
radioshaqand passwordradioshaq. While this is only a default for local development, having credentials in source code makes it easy for them to leak into production if the env var override is missed. Consider using a default URL without credentials (e.g., just the connection string structure) or documenting this more prominently.radioshaq/web-interface/src/services/radioshaqApi.ts, line 18-21 (link)API token baked into every frontend bundle via
VITE_environment variableVITE_RADIOSHAQ_TOKENis embedded at build time into the JavaScript bundle by Vite (allVITE_*env vars are inlined). This means any user who opens the browser dev tools can extract the Bearer token from the compiled JS source. If this token has elevated privileges (e.g., admin/operator role), it grants those privileges to anyone who inspects the page.For a local-only dev tool this may be acceptable, but the code structure (auth headers on every request, SSE streams, emergency approve/reject) suggests this is intended to be deployed to real operator stations over a network. In that case, the token should be acquired via a proper login flow (the
getTokenendpoint already exists at line 29) and stored in memory only, not baked into the bundle.Recommendation: Remove
VITE_RADIOSHAQ_TOKENfrom the build-time env. Use the existinggetToken+setApiTokenruntime flow exclusively. If a shortcut is needed for development, at least document that it must never be used in production.radioshaq/radioshaq/api/routes/bus.py, line 24-28 (link)No authentication on inbound bus endpoint
POST /bus/inboundhas noDepends(get_current_user)dependency, unlike theopt_outendpoint in the same file (line 69). Any network-reachable client can inject arbitrary messages into the message bus and orchestrator pipeline without authentication.If this is intentional for Lambda-to-API communication, consider adding an internal service verification or network-level restriction. Otherwise, add JWT auth similar to the
opt_outendpoint.Last reviewed commit: 4960690