Conversation
Owner
Author
|
@greptileai : update your review based on the fixes above : |
Owner
Author
|
@greptileai , update your review based on the changes above : |
Owner
Author
|
@greptileai : update your review based on the recent changes : |
Owner
Author
|
@greptileai : update your review based on the recent changes made : |
Josephrp
added a commit
that referenced
this pull request
Mar 9, 2026
* Enforce `main <- dev` governance, migrate to GPL-2.0-only, add CLI/UI license gates, and split stable/nightly PyPI release lanes (#16) * adds memory system (#6) * adds multiband and relay support (#7) * Add frequency-aware radio replies (#8) * Fixes PyPI UI bundling flow (#12) * fixes build env bug (#13) * fixes ci issue (#14) * adds memory system (#6) * adds multiband and relay support (#7) * Add frequency-aware radio replies (#8) * Fixes PyPI UI bundling flow (#12) * fixes build env bug (#13) * fixes ci issue (#14) * readme improvements and readthedocs ci (#22) * adds language support and interface improvements (#23) * GIS location capture flow, dependency guidance, and test fixes (#25) * Gis (#27) * adds Country-specific compliance plugin (FCC, CEPT, R1 band plans) (#28) * TTS/ASR provider plugin, Hugging Face Inference, local options, .gitignore cleanup (#31) * SMS/WhatsApp relay, emergency approval, contact preferences & testing improvements (#32) * Patch (#33)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
solves #9
Summary
This PR adds SMS and WhatsApp support (Twilio), emergency message approval (operator-in-the-loop), contact preferences and notify-on-relay (per-channel opt-out), consolidates docs into a single Response & compliance page, and improves CI and test reliability (Postgres + migrations, test fixes).
Latest commits:
1. SMS & WhatsApp (Twilio)
twilio.account_sid,twilio.auth_token,twilio.from_number,twilio.whatsapp_from(envRADIOSHAQ_TWILIO__*or YAML). Documented in Configuration → Twilio.client=toWhatsAppAgent(fixed parameter name).radioshaq setup --reconfigurecaptures Twilio credentials (including auth token) before clearing secrets and writes them to.env; TTS reconfigure path now returns and persists the ElevenLabs API key.2. Emergency approval (operator receive & transmit)
RADIOSHAQ_EMERGENCY_CONTACT__ENABLED,RADIOSHAQ_EMERGENCY_CONTACT__REGIONS_ALLOWED. See Response & compliance.GET /emergency/pending-count,GET /emergency/events,GET /emergency/events/stream(SSE)POST /emergency/events/{id}/approve,POST /emergency/events/{id}/rejectemergency: trueand SMS/WhatsApp target queues for approval; API route returnsqueued_for_approval+event_id(no KeyError).extra_dataupdates use a new dict so SQLAlchemy persists audit fields (approved_at,approved_by,sent_at,rejected_at). Relay service and orchestrator relay tool receive fullConfigforemergency_contact.3. Relay (radio + SMS/WhatsApp)
target_channelissmsorwhatsapp,target_bandis not looked up in band plans (avoids KeyError);target_freqdefault 0 for non-radio.publish_outboundreturns true (avoids marking delivered when queue is full; failed items stay pending for retry).4. Contact preferences & per-channel opt-out
GET/PATCH /callsigns/registered/{callsign}/contact-preferencesfor notify-on-relay (SMS/WhatsApp phones, consent). New migration addsnotify_opt_out_at_smsandnotify_opt_out_at_whatsapp.record_opt_out(callsign, channel)andrecord_opt_out_by_phone(phone, channel)set only the channel-specific timestamp and clear that channel’s phone. No single globalnotify_opt_out_atfor the worker.notify_opt_out_at_sms/notify_opt_out_at_whatsappper channel; opting out of SMS does not block WhatsApp and vice versa.set_contact_preferencesclearsnotify_opt_out_at_smswhen a non-empty SMS phone is set, andnotify_opt_out_at_whatsappwhen a non-empty WhatsApp phone is set.5. Documentation
monitoring.md; nav entry is now “Response & compliance”. compliance-regulatory.md is a short stub pointing to that page.band_plan_regiondocumented in Configuration. API reference updated (GIS, emergency, metrics link).6. CI & test harness
test-ci,publish-nightly,publish-pypiuse a Postgres service (port 5434), Wait for Postgres, and Run database migrations before tests so the schema (includingnotify_opt_out_at_sms/notify_opt_out_at_whatsapp) is current.clientfixture triggers a session-scoped migration run (_run_db_migrations) so DB-using tests (e.g. callsigns) see the latest schema; if migrations fail (e.g. no Postgres), the session is skipped.get_contact_preferencescalls (worker loop) are allowed; still asserts at least one call with the destination callsign._run_reconfigure_promptsnow returns 11 values (includingelevenlabs_key_reconfigure); mock intest_run_setup_reconfigure_mocked_merges_configupdated accordingly.7. Other fixes and tweaks
WhatsAppAgentis constructed withclient=sms_client(nottwilio_client) to matchWhatsAppAgent.__init__(client=..., from_number=...).write_env(elevenlabs_api_key=...)so the key is not lost.queued_for_approval, the route returnsok,queued_for_approval,event_id,target_channelinstead of accessing missing keys.extra_data:update_coordination_eventusesdict(row.extra_data or {})so SQLAlchemy detects the change and persists audit fields..ruff_cache/,.tmp_build/,.tmp_pytest/,dist-investigate/; cache/temp patterns; “RadioShaq” comment. LICENSE.md moved to repo root (GPL text only).Migration and compatibility
alembic upgrade head(oruv run alembic-upgrade) soregistered_callsignshasnotify_opt_out_at_smsandnotify_opt_out_at_whatsapp. Existing rows keepnotify_opt_out_at; legacy behaviour is “both channels opted out” when that timestamp is set and per-channel columns are null.emergency_contact,twilio,tts; existing configs remain valid. Optional: setTEST_DATABASE_URLfor tests if not using defaultpostgresql+asyncpg://...@127.0.0.1:5434/radioshaq.How to test
radioshaq/:uv run pytest tests/unit tests/integration -v. Requires Postgres on port 5434 (or setTEST_DATABASE_URL) for tests that use the DB; migrations run automatically when theclientfixture is used.emergency: trueand SMS/WhatsApp target; use Emergency page or API to approve/reject.notify_on_relay: true; trigger a radio relay for that callsign and confirm a short SMS/WhatsApp is sent (Twilio configured). Opt out viaPOST /internal/opt-outand confirm only that channel stops.Checklist
queued_for_approvalGreptile Summary
This PR adds SMS/WhatsApp delivery via Twilio, an operator-in-the-loop emergency approval workflow, per-channel notify-on-relay opt-out, and consolidates CI and docs. The overall architecture is solid and many previously-flagged issues (TOCTOU in approve, AudioContext leak, E.164 normalization,
stop_eventblocking, opt-out auth, stuck-event-on-failed-delivery) have been correctly addressed.Critical issues remain:
Events permanently stuck in
"approving"state (postgres_gis.py:583) —get_pending_coordination_eventsonly queriesstatus == "pending". If the server crashes afterclaim_emergency_event_pendingcommits but before the final state change completes, the event stays"approving"forever and is invisible to all operators with no recovery path.Legacy
notify_opt_out_atblocks per-channel re-opt-in (postgres_gis.py:763-764) —get_contact_preferencescomputesopt_out_sms = row.notify_opt_out_at_sms or row.notify_opt_out_at.set_contact_preferencesclears only the per-channel column but never the legacy global column, so users who used the old opt-out mechanism can never successfully re-opt-in to SMS or WhatsApp notifications.record_opt_out_by_phoneraises unhandledMultipleResultsFound(postgres_gis.py:845) — Phone number columns have noUNIQUEconstraint;scalar_one_or_none()raisessqlalchemy.exc.MultipleResultsFoundif two callsigns share a phone, producing a 500 error and silently skipping the opt-out — a TCPA/GDPR compliance risk.Reject endpoint lacks atomic claim (
emergency.py:239-250) —reject_emergency_eventreads, checks, then writes without usingclaim_emergency_event_pending. A concurrent approve+reject can both complete, leaving a delivered message recorded as"rejected".Confidence Score: 1/5
postgres_gis.py(events stuck in "approving" state with no recovery, legacy global opt-out permanently blocking per-channel re-opt-in, unhandledMultipleResultsFoundexception in opt-out by phone) directly affect the correctness and compliance guarantees of the new opt-out and re-opt-in flows. Additionally, the reject endpoint lacks the atomic claim guard that was added to approve, allowing a race that can result in a sent message being recorded as rejected. These are production-critical issues for a system handling GDPR/TCPA-regulated messaging.Sequence Diagram
sequenceDiagram participant Operator participant EmergencyAPI participant DB participant MessageBus participant Twilio Operator->>EmergencyAPI: POST /emergency/events/{id}/approve EmergencyAPI->>DB: claim_emergency_event_pending(id)<br/>(atomic: pending → approving) DB-->>EmergencyAPI: claimed (or None if already processed) alt Claim failed EmergencyAPI-->>Operator: 400 Event already processed else Claim succeeded EmergencyAPI->>MessageBus: publish_outbound(channel, phone, message) alt Queue full / bus unavailable EmergencyAPI->>DB: update status → "pending" (rollback) EmergencyAPI-->>Operator: 200 sent=false (retry) else Published OK EmergencyAPI->>DB: update status → "approved" + audit fields MessageBus->>Twilio: SMS / WhatsApp send EmergencyAPI-->>Operator: 200 sent=true end end Note over EmergencyAPI,DB: Reject path does NOT use claim_emergency_event_pending<br/>(race: concurrent approve+reject can both succeed) Operator->>EmergencyAPI: POST /emergency/events/{id}/reject EmergencyAPI->>DB: get_coordination_event_by_id(id) DB-->>EmergencyAPI: event (status may already be "approving") EmergencyAPI->>DB: update status → "rejected" (no atomic guard)Comments Outside Diff (2)
radioshaq/radioshaq/specialized/relay_tools.py, line 100-104 (link)target_bandlisted asrequiredeven for SMS/WhatsApp channelThe JSON schema for the
relay_message_between_bandstool still declarestarget_bandas required in the"required"array:When
target_channelis"sms"or"whatsapp",target_bandis unused by the service (the service skips band-plan lookup for non-radio channels). However, the LLM agent reading this schema will always be prompted to supply atarget_band, even for SMS/WhatsApp relays where it has no meaningful value to provide. This can confuse the model and produce dummy values like"sms"or"unknown"for what is described as a required radio band.Consider making
target_bandoptional whentarget_channelis not"radio":Or document in the description that
target_bandcan be any placeholder value (e.g."n/a") whentarget_channelissms/whatsapp.radioshaq/radioshaq/database/postgres_gis.py, line 581-584 (link)Events permanently stuck in
"approving"state after crashget_pending_coordination_events(line 583) hard-codes.where(CoordinationEvent.status == "pending"). Events transition to"approving"viaclaim_emergency_event_pendingduring approval and are supposed to transition back to"pending"ifpublish_outboundfails, or to"approved"on success.If the server crashes after
claim_emergency_event_pendingcommitsstatus="approving"but before the rollback (when publish fails) or the success write completes, the row stays at"approving"indefinitely. Becauseget_pending_coordination_eventsonly returnsstatus == "pending"rows, such events become completely invisible to operators — there is no UI, endpoint, or cleanup job that can surface or recover them.At minimum, consider also querying for
"approving"events that are older than a short timeout (e.g. 5 minutes) and resetting them to"pending":Alternatively, a startup or periodic cleanup task could reset stale
"approving"events back to"pending".Last reviewed commit: b8e734c