From 10b349ef33b26fff08f8e6d48cf09c1076869fef Mon Sep 17 00:00:00 2001 From: Adamskiee Date: Tue, 11 Aug 2026 13:46:08 +0800 Subject: [PATCH 01/15] fix(gsm-queue): bound outbound SMS admission --- GSM-module/GSM-fastapi/.env.example | 3 + GSM-module/GSM-fastapi/api.py | 43 ++++- GSM-module/GSM-fastapi/config.py | 20 +++ GSM-module/GSM-fastapi/serial_worker.py | 148 ++++++++++++------ GSM-module/GSM-fastapi/tests/conftest.py | 6 + .../GSM-fastapi/tests/test_api_queue.py | 79 ++++++++++ GSM-module/GSM-fastapi/tests/test_config.py | 23 +++ .../GSM-fastapi/tests/test_serial_worker.py | 102 ++++++++++++ deploy/config/gsm-fastapi.env.example | 3 + 9 files changed, 379 insertions(+), 48 deletions(-) create mode 100644 GSM-module/GSM-fastapi/tests/conftest.py create mode 100644 GSM-module/GSM-fastapi/tests/test_api_queue.py create mode 100644 GSM-module/GSM-fastapi/tests/test_config.py create mode 100644 GSM-module/GSM-fastapi/tests/test_serial_worker.py diff --git a/GSM-module/GSM-fastapi/.env.example b/GSM-module/GSM-fastapi/.env.example index dd03f194..3d328628 100644 --- a/GSM-module/GSM-fastapi/.env.example +++ b/GSM-module/GSM-fastapi/.env.example @@ -8,6 +8,9 @@ HOST=127.0.0.1 # to avoid a port collision. PORT=8000 LOG_LEVEL=INFO +# Maximum outbound SMS requests waiting behind the one in-flight request. +# Must be an integer greater than or equal to 1. +SMS_SEND_QUEUE_MAXSIZE=10 SAPOT_API_URL=https://localhost:8000 GSM_SECRET=change-me-to-a-strong-secret SMS_BOT_USER_ID= diff --git a/GSM-module/GSM-fastapi/api.py b/GSM-module/GSM-fastapi/api.py index d7bc9b73..37e06d6a 100644 --- a/GSM-module/GSM-fastapi/api.py +++ b/GSM-module/GSM-fastapi/api.py @@ -38,7 +38,11 @@ import database from app_version import __version__ -from serial_worker import SerialWorker +from serial_worker import ( + OutboundQueueFullError, + SerialWorker, + WorkerStoppingError, +) from sms_handler import handle_incoming_sms from config import settings @@ -59,7 +63,11 @@ async def lifespan(app: FastAPI): logger.info("Database ready") # Serial worker - _worker = SerialWorker(settings.serial_port, settings.serial_baud) + _worker = SerialWorker( + settings.serial_port, + settings.serial_baud, + settings.sms_send_queue_maxsize, + ) _worker.start() logger.info("Serial worker started on %s", settings.serial_port) @@ -153,6 +161,12 @@ def _send_and_log(from_number: str, to_number: str, body: str): result = _worker.send_sms(to_number, body, timeout=120) status = "sent" if result["ok"] else "failed" database.update_message_status(msg_id, status, result.get("reason")) + except OutboundQueueFullError: + logger.warning("Internal outbound SMS rejected: QUEUE_FULL") + database.update_message_status(msg_id, "failed", "QUEUE_FULL") + except WorkerStoppingError: + logger.info("Internal outbound SMS rejected: SERVICE_STOPPING") + database.update_message_status(msg_id, "failed", "SERVICE_STOPPING") except Exception as e: logger.error("send_sms error: %s", e) database.update_message_status(msg_id, "failed", str(e)) @@ -206,7 +220,7 @@ def validate_phone(cls, v): # ── Health endpoints ────────────────────────────────────────────────────────── @app.get("/health", tags=["health"]) -def health(): +async def health(): """ Liveness check. Returns 200 if the API is running. Returns 503 if the GSM modem is not ready (so load balancers can react). @@ -257,6 +271,9 @@ def pct(value): "connected": _worker.connected, "last_status": _worker.last_status, "queue_depth": _worker.incoming_queue.qsize(), + "outbound_queue_depth": _worker.outbound_queue_depth, + "outbound_queue_capacity": _worker.outbound_queue_capacity, + "outbound_in_flight": _worker.outbound_in_flight, "port": settings.serial_port, "baud": settings.serial_baud, "total_messages": total, @@ -323,6 +340,20 @@ def send_sms(req: SendSMSRequest): try: result = _worker.send_sms(req.number, req.body, timeout=60) + except OutboundQueueFullError: + database.update_message_status(msg_id, "failed", "QUEUE_FULL") + raise HTTPException(503, { + "message": "Outbound SMS queue is full", + "reason": "QUEUE_FULL", + "msg_id": msg_id, + }) + except WorkerStoppingError: + database.update_message_status(msg_id, "failed", "SERVICE_STOPPING") + raise HTTPException(503, { + "message": "SMS service is stopping", + "reason": "SERVICE_STOPPING", + "msg_id": msg_id, + }) except RuntimeError as e: database.update_message_status(msg_id, "failed", str(e)) raise HTTPException(503, str(e)) @@ -331,6 +362,12 @@ def send_sms(req: SendSMSRequest): database.update_message_status(msg_id, status, result.get("reason")) if not result["ok"]: + if result["reason"] == "SERVICE_STOPPING": + raise HTTPException(503, { + "message": "SMS service is stopping", + "reason": "SERVICE_STOPPING", + "msg_id": msg_id, + }) raise HTTPException(502, { "message": "SMS delivery failed", "reason": result["reason"], diff --git a/GSM-module/GSM-fastapi/config.py b/GSM-module/GSM-fastapi/config.py index d78c411c..3d279489 100644 --- a/GSM-module/GSM-fastapi/config.py +++ b/GSM-module/GSM-fastapi/config.py @@ -13,6 +13,23 @@ from dotenv import load_dotenv +def positive_integer_env(name: str, default: int) -> int: + value = os.environ.get(name) + if value is None: + return default + try: + parsed = int(value) + except ValueError as error: + raise RuntimeError( + f"Environment variable '{name}' must be an integer greater than or equal to 1." + ) from error + if parsed < 1: + raise RuntimeError( + f"Environment variable '{name}' must be an integer greater than or equal to 1." + ) + return parsed + + class Settings: load_dotenv() # Serial port the Arduino is connected to @@ -34,5 +51,8 @@ class Settings: # Logging level log_level: str = os.environ.get("LOG_LEVEL", "INFO") + # Maximum number of outbound SMS requests waiting behind the in-flight send + sms_send_queue_maxsize: int = positive_integer_env("SMS_SEND_QUEUE_MAXSIZE", 10) + settings = Settings() diff --git a/GSM-module/GSM-fastapi/serial_worker.py b/GSM-module/GSM-fastapi/serial_worker.py index 0457f96d..417ac6fe 100644 --- a/GSM-module/GSM-fastapi/serial_worker.py +++ b/GSM-module/GSM-fastapi/serial_worker.py @@ -36,6 +36,14 @@ RECONNECT_DELAY = 10 # seconds between reconnect attempts +class OutboundQueueFullError(RuntimeError): + """Raised when no outbound waiting slot is available.""" + + +class WorkerStoppingError(RuntimeError): + """Raised when a request arrives after the shutdown admission cutoff.""" + + @dataclass class _SendRequest: """One pending send_sms() call, passed through the internal queue.""" @@ -59,7 +67,8 @@ class SerialWorker: Arduino confirms → reader thread resolves _SendRequest → caller unblocks """ - def __init__(self, port: str, baud: int = 9600): + def __init__(self, port: str, baud: int = 9600, + send_queue_maxsize: int = 10): self._port = port self._baud = baud @@ -67,9 +76,12 @@ def __init__(self, port: str, baud: int = 9600): self._ser_lock = threading.Lock() # guards writes to _ser self._stop = threading.Event() + self._lifecycle_lock = threading.Lock() + self._accepting = True + self._stop_lock = threading.Lock() - # Outbound queue: send_sms() puts requests here; sender thread consumes - self._send_queue: queue.Queue[_SendRequest] = queue.Queue() + self._send_queue: queue.Queue[_SendRequest] = queue.Queue( + maxsize=send_queue_maxsize) # The one request currently being sent (set by sender, read by reader) self._in_flight: Optional[_SendRequest] = None @@ -95,11 +107,27 @@ def start(self): self._sender_thread.start() def stop(self): - self._stop.set() - # Unblock sender thread if waiting on empty queue - self._send_queue.put(_SendRequest(number="", body="", timeout=0)) - self._reader_thread.join(timeout=5) - self._sender_thread.join(timeout=5) + """Stop admission first, then preserve the current send's true result.""" + with self._stop_lock: + if self._stop.is_set(): + return + + with self._lifecycle_lock: + self._accepting = False + + self._drain_queued_requests() + + # Once admission is closed, a sender can no longer move a dequeued + # request into flight. A request observed here is therefore the only + # one that can still reach the serial port. + with self._in_flight_lock: + in_flight = self._in_flight + if in_flight is not None: + in_flight.done.wait() + + self._stop.set() + self._reader_thread.join(timeout=5) + self._sender_thread.join(timeout=5) # ── Public API ──────────────────────────────────────────────────────────── @@ -114,13 +142,21 @@ def send_sms(self, number: str, body: str, Returns {"ok": bool, "reason": str|None} Raises RuntimeError if modem not ready or port not connected. """ - if not self.connected: - raise RuntimeError("Serial port not connected") - if not self.gsm_ready: - raise RuntimeError("GSM modem not ready") - req = _SendRequest(number=number, body=body, timeout=timeout) - self._send_queue.put(req) + with self._lifecycle_lock: + if not self._accepting: + raise WorkerStoppingError("SMS service is stopping") + if not self.connected: + raise RuntimeError("Serial port not connected") + if not self.gsm_ready: + raise RuntimeError("GSM modem not ready") + try: + self._send_queue.put_nowait(req) + except queue.Full as error: + logger.warning( + "Outbound SMS queue full (depth=%d capacity=%d)", + self._send_queue.qsize(), self.outbound_queue_capacity) + raise OutboundQueueFullError("Outbound SMS queue is full") from error logger.info("SMS enqueued to %s (queue depth %d)", number, self._send_queue.qsize()) @@ -130,6 +166,19 @@ def send_sms(self, number: str, body: str, return {"ok": False, "reason": "CLIENT_TIMEOUT"} return {"ok": req.success, "reason": req.reason} + @property + def outbound_queue_depth(self) -> int: + return self._send_queue.qsize() + + @property + def outbound_queue_capacity(self) -> int: + return self._send_queue.maxsize + + @property + def outbound_in_flight(self) -> bool: + with self._in_flight_lock: + return self._in_flight is not None + # ── Sender thread: one at a time, in order ──────────────────────────────── def _sender_loop(self): @@ -144,8 +193,13 @@ def _sender_loop(self): except queue.Empty: continue - if self._stop.is_set() or not req.number: - break # sentinel or stop + with self._lifecycle_lock: + if not self._accepting: + self._complete_request(req, False, "SERVICE_STOPPING") + continue + # Register before writing so the reader never misses an event. + with self._in_flight_lock: + self._in_flight = req # Wait until modem is ready (e.g. after reconnect) deadline = time.time() + req.timeout @@ -153,17 +207,10 @@ def _sender_loop(self): time.sleep(0.5) if not self.gsm_ready: - req.success = False - req.reason = "MODEM_NOT_READY" - req.done.set() + self._complete_in_flight(req, False, "MODEM_NOT_READY") logger.warning("SMS to %s dropped — modem not ready", req.number) continue - # Register as in-flight BEFORE writing to serial, - # so the reader never misses the SMS_SENT event - with self._in_flight_lock: - self._in_flight = req - cmd = build_send_sms(req.number, req.body) try: with self._ser_lock: @@ -175,24 +222,15 @@ def _sender_loop(self): raise OSError("Serial port not open") except Exception as e: logger.error("Serial write failed: %s", e) - with self._in_flight_lock: - self._in_flight = None - req.success = False - req.reason = f"WRITE_ERROR: {e}" - req.done.set() + self._complete_in_flight(req, False, f"WRITE_ERROR: {e}") continue # Block here until the reader resolves this request # (or until the per-SMS timeout expires) resolved = req.done.wait(timeout=req.timeout) if not resolved: - with self._in_flight_lock: - if self._in_flight is req: - self._in_flight = None - req.success = False - req.reason = "TIMEOUT" - req.done.set() - logger.error("SMS to %s timed out", req.number) + if self._complete_in_flight(req, False, "TIMEOUT"): + logger.error("SMS to %s timed out", req.number) # ── Reader thread: serial → events ──────────────────────────────────────── @@ -220,7 +258,8 @@ def _reader_loop(self): def _connect_and_read(self): logger.info("Opening %s @ %d baud", self._port, self._baud) try: - ser = serial.Serial(self._port, self._baud, timeout=1) + ser = serial.Serial(self._port, self._baud, timeout=1, + write_timeout=5.0) except serial.SerialException as e: logger.error("Cannot open %s: %s", self._port, e) self.last_status = f"port error: {e}" @@ -327,19 +366,38 @@ def _resolve_in_flight(self, number: str, success: bool, "SMS_SENT/FAILED number mismatch: expected %s got %s", req.number, number) # Still resolve it — the Arduino only handles one at a time - self._in_flight = None - - req.success = success - req.reason = reason - req.done.set() + self._complete_in_flight(req, success, reason) def _fail_in_flight(self, reason: str): with self._in_flight_lock: req = self._in_flight if req is None: return + if self._complete_in_flight(req, False, reason): + logger.warning("In-flight SMS to %s failed: %s", req.number, reason) + + def _complete_in_flight(self, req: _SendRequest, success: bool, + reason: Optional[str]) -> bool: + with self._in_flight_lock: + if self._in_flight is not req: + return False self._in_flight = None - req.success = False - req.reason = reason + req.success = success + req.reason = reason + req.done.set() + return True + + @staticmethod + def _complete_request(req: _SendRequest, success: bool, + reason: Optional[str]): + req.success = success + req.reason = reason req.done.set() - logger.warning("In-flight SMS to %s failed: %s", req.number, reason) + + def _drain_queued_requests(self): + while True: + try: + req = self._send_queue.get_nowait() + except queue.Empty: + return + self._complete_request(req, False, "SERVICE_STOPPING") diff --git a/GSM-module/GSM-fastapi/tests/conftest.py b/GSM-module/GSM-fastapi/tests/conftest.py new file mode 100644 index 00000000..7ad3262d --- /dev/null +++ b/GSM-module/GSM-fastapi/tests/conftest.py @@ -0,0 +1,6 @@ +import os +import sys +from pathlib import Path + +os.environ.setdefault("DB_PATH", "sqlite:///./test-gsm.db") +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) diff --git a/GSM-module/GSM-fastapi/tests/test_api_queue.py b/GSM-module/GSM-fastapi/tests/test_api_queue.py new file mode 100644 index 00000000..b71b4fc4 --- /dev/null +++ b/GSM-module/GSM-fastapi/tests/test_api_queue.py @@ -0,0 +1,79 @@ +from fastapi.testclient import TestClient +import pytest + +import api +from serial_worker import OutboundQueueFullError, WorkerStoppingError + + +@pytest.fixture(autouse=True) +def reset_worker(): + previous = api._worker + yield + api._worker = previous + + +class RejectingWorker: + gsm_ready = True + connected = True + last_status = "ready" + outbound_queue_depth = 1 + outbound_queue_capacity = 1 + outbound_in_flight = True + + def __init__(self, error): + self.error = error + + def send_sms(self, *_args, **_kwargs): + raise self.error + + +def test_queue_full_returns_nested_503_and_updates_log(monkeypatch): + updates = [] + monkeypatch.setattr(api.database, "log_message", lambda **_kwargs: "message-id") + monkeypatch.setattr(api.database, "update_message_status", lambda *args: updates.append(args)) + api._worker = RejectingWorker(OutboundQueueFullError()) + + response = TestClient(api.app).post("/sms/send", json={ + "number": "+639171234567", "body": "message" + }) + + assert response.status_code == 503 + assert response.json()["detail"] == { + "message": "Outbound SMS queue is full", + "reason": "QUEUE_FULL", + "msg_id": "message-id", + } + assert updates == [("message-id", "failed", "QUEUE_FULL")] + + +def test_stopping_returns_nested_503_and_updates_log(monkeypatch): + updates = [] + monkeypatch.setattr(api.database, "log_message", lambda **_kwargs: "message-id") + monkeypatch.setattr(api.database, "update_message_status", lambda *args: updates.append(args)) + api._worker = RejectingWorker(WorkerStoppingError()) + + response = TestClient(api.app).post("/sms/send", json={ + "number": "+639171234567", "body": "message" + }) + + assert response.status_code == 503 + assert response.json()["detail"]["reason"] == "SERVICE_STOPPING" + assert updates == [("message-id", "failed", "SERVICE_STOPPING")] + + +def test_detailed_health_keeps_inbound_queue_depth_and_adds_outbound_fields(monkeypatch): + class Worker(RejectingWorker): + def __init__(self): + self.incoming_queue = __import__("queue").Queue() + self.incoming_queue.put(object()) + + api._worker = Worker() + monkeypatch.setattr(api.database, "get_messages", lambda **_kwargs: {"messages": [], "total": 0}) + + response = TestClient(api.app).get("/health/detailed") + + assert response.status_code == 200 + assert response.json()["queue_depth"] == 1 + assert response.json()["outbound_queue_depth"] == 1 + assert response.json()["outbound_queue_capacity"] == 1 + assert response.json()["outbound_in_flight"] is True diff --git a/GSM-module/GSM-fastapi/tests/test_config.py b/GSM-module/GSM-fastapi/tests/test_config.py new file mode 100644 index 00000000..05b9ce2b --- /dev/null +++ b/GSM-module/GSM-fastapi/tests/test_config.py @@ -0,0 +1,23 @@ +import pytest + +from config import positive_integer_env + + +def test_positive_integer_env_uses_default_when_missing(monkeypatch): + monkeypatch.delenv("TEST_QUEUE_SIZE", raising=False) + + assert positive_integer_env("TEST_QUEUE_SIZE", 10) == 10 + + +@pytest.mark.parametrize("value", ["0", "-1", "", "ten"]) +def test_positive_integer_env_rejects_invalid_values(monkeypatch, value): + monkeypatch.setenv("TEST_QUEUE_SIZE", value) + + with pytest.raises(RuntimeError, match="TEST_QUEUE_SIZE.*integer"): + positive_integer_env("TEST_QUEUE_SIZE", 10) + + +def test_positive_integer_env_accepts_positive_value(monkeypatch): + monkeypatch.setenv("TEST_QUEUE_SIZE", "4") + + assert positive_integer_env("TEST_QUEUE_SIZE", 10) == 4 diff --git a/GSM-module/GSM-fastapi/tests/test_serial_worker.py b/GSM-module/GSM-fastapi/tests/test_serial_worker.py new file mode 100644 index 00000000..fd04de16 --- /dev/null +++ b/GSM-module/GSM-fastapi/tests/test_serial_worker.py @@ -0,0 +1,102 @@ +import pytest + +from serial_worker import ( + OutboundQueueFullError, + SerialWorker, + WorkerStoppingError, + _SendRequest, +) + + +def ready_worker(capacity=1): + worker = SerialWorker("fake", send_queue_maxsize=capacity) + worker.connected = True + worker.gsm_ready = True + return worker + + +def test_capacity_rejects_without_serial_write(): + worker = ready_worker() + worker._send_queue.put(_SendRequest("+639171234567", "first", 1)) + + with pytest.raises(OutboundQueueFullError): + worker.send_sms("+639171234568", "second", timeout=0) + + assert worker.outbound_queue_depth == 1 + assert worker.outbound_in_flight is False + + +def test_capacity_excludes_registered_in_flight_request(): + worker = ready_worker() + in_flight = _SendRequest("+639171234567", "first", 1) + with worker._in_flight_lock: + worker._in_flight = in_flight + + worker._send_queue.put_nowait(_SendRequest("+639171234568", "second", 1)) + + assert worker.outbound_in_flight is True + assert worker.outbound_queue_depth == 1 + assert worker.outbound_queue_capacity == 1 + + +def test_stop_drains_waiting_requests_without_sentinel(): + worker = ready_worker() + request = _SendRequest("+639171234567", "queued", 1) + worker._send_queue.put_nowait(request) + class JoinedThread: + def join(self, timeout): + assert timeout == 5 + + worker._reader_thread = JoinedThread() + worker._sender_thread = JoinedThread() + + worker.stop() + + assert request.done.is_set() + assert request.reason == "SERVICE_STOPPING" + assert worker.outbound_queue_depth == 0 + + +def test_admission_rejects_after_shutdown_cutoff(): + worker = ready_worker() + with worker._lifecycle_lock: + worker._accepting = False + + with pytest.raises(WorkerStoppingError): + worker.send_sms("+639171234567", "message", timeout=0) + + +def test_in_flight_completion_is_exact_once(): + worker = ready_worker() + request = _SendRequest("+639171234567", "message", 1) + with worker._in_flight_lock: + worker._in_flight = request + + assert worker._complete_in_flight(request, True, None) is True + assert worker._complete_in_flight(request, False, "TIMEOUT") is False + assert request.success is True + assert request.reason is None + + +def test_serial_connection_uses_finite_write_timeout(monkeypatch): + captured = {} + + class FakeSerial: + is_open = True + in_waiting = 0 + + def __init__(self, *args, **kwargs): + captured.update(kwargs) + + def read(self, _): + worker._stop.set() + return b"" + + def close(self): + pass + + worker = SerialWorker("fake") + monkeypatch.setattr("serial_worker.serial.Serial", FakeSerial) + worker._connect_and_read() + + assert captured["write_timeout"] == 5.0 diff --git a/deploy/config/gsm-fastapi.env.example b/deploy/config/gsm-fastapi.env.example index 38586915..8db25e8c 100644 --- a/deploy/config/gsm-fastapi.env.example +++ b/deploy/config/gsm-fastapi.env.example @@ -4,6 +4,9 @@ DB_PATH=mysql+pymysql://sapot:__FROM_SERVER_MYSQL_PASSWORD__@db:3306/sapot HOST=0.0.0.0 PORT=8001 LOG_LEVEL=INFO +# Maximum outbound SMS requests waiting behind the one in-flight request. +# Must be an integer greater than or equal to 1. +SMS_SEND_QUEUE_MAXSIZE=10 SAPOT_API_URL=https://nginx GSM_SECRET=__FROM_SERVER_GSM_SECRET__ SMS_BOT_USER_ID= From 7347f522bb7335a2e1721bea30fb244ebccbeb1e Mon Sep 17 00:00:00 2001 From: Adamskiee Date: Tue, 11 Aug 2026 13:46:08 +0800 Subject: [PATCH 02/15] chore(deploy-gsm): align GSM shutdown grace periods --- deployment-scripts/server-GSM-api.service | 1 + docker-compose.prod.yml | 1 + docker-compose.yml | 1 + 3 files changed, 3 insertions(+) diff --git a/deployment-scripts/server-GSM-api.service b/deployment-scripts/server-GSM-api.service index b445e8ef..f0f5f44f 100644 --- a/deployment-scripts/server-GSM-api.service +++ b/deployment-scripts/server-GSM-api.service @@ -8,6 +8,7 @@ Group=sapot WorkingDirectory=/home/sapot/YLP-software/GSM-module/GSM-fastapi ExecStart=/home/sapot/YLP-software/GSM-module/GSM-fastapi/run-api.sh Restart=always +TimeoutStopSec=150 # Automatic restart logic RestartSec=3 diff --git a/docker-compose.prod.yml b/docker-compose.prod.yml index 88e8725e..a5748d57 100644 --- a/docker-compose.prod.yml +++ b/docker-compose.prod.yml @@ -70,6 +70,7 @@ services: gsm-fastapi: image: sapot/gsm-fastapi:bundle restart: unless-stopped + stop_grace_period: 150s env_file: [../../../shared/gsm-fastapi.env] environment: HOST: 0.0.0.0 diff --git a/docker-compose.yml b/docker-compose.yml index 0eaab2eb..196827b0 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -121,6 +121,7 @@ services: build: context: ./GSM-module/GSM-fastapi restart: unless-stopped + stop_grace_period: 150s env_file: - ./GSM-module/GSM-fastapi/.env environment: From 003de2e5be144bbc4808b1bb008dc3edd4171be8 Mon Sep 17 00:00:00 2001 From: Adamskiee Date: Tue, 11 Aug 2026 13:46:08 +0800 Subject: [PATCH 03/15] docs(docs-gsm): document outbound queue saturation --- CLAUDE.md | 2 +- GSM-module/CLAUDE.md | 7 +- docs/deployment/environment-config.md | 5 +- docs/deployment/gsm-module.md | 38 ++++++----- docs/deployment/maintenance.md | 2 +- docs/deployment/monitoring-logging.md | 2 + docs/features/sms-gateway/design.md | 78 ++++++++--------------- docs/features/sms-gateway/requirements.md | 27 ++++++-- docs/features/sms-gateway/testing.md | 34 +++++++--- 9 files changed, 103 insertions(+), 92 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 5772d520..980152e3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -60,7 +60,7 @@ Run for each component actually touched — don't assume one component's green b | `server/app/models/` changed | `alembic upgrade head && alembic check` — **run from `server/`** (not `server/app/`) with `DATABASE_URL` set. `alembic check` must report no new operations. Note `pytest` builds its schema with `create_all()` and cannot detect migration drift. | | `mobile-app/sapot-mobile-app/` | `pnpm run testAll` (= test + typecheck + lint + expo-doctor), or the individual `pnpm test` / `pnpm run typecheck` / `pnpm run lint` | | `admin-frontend/sapot-admin/` | `pnpm run lint && pnpm run build` — **no test script exists in this component**; don't claim test coverage that isn't there | -| `GSM-module/` | No automated tests exist — verify manually per `docs/getting-started/gsm-module-setup.md` | +| `GSM-module/` | `pytest` (from `GSM-module/GSM-fastapi/`; serial I/O and database calls are mocked) | If the change is release-relevant (server), `server/app/version.py` must match the git tag per `VERSIONING.md` before tagging — not typically a per-commit concern, but relevant if asked to prepare a release. diff --git a/GSM-module/CLAUDE.md b/GSM-module/CLAUDE.md index 27703143..9d328a88 100644 --- a/GSM-module/CLAUDE.md +++ b/GSM-module/CLAUDE.md @@ -8,17 +8,17 @@ Three layers: Arduino firmware talking AT commands to a SIM800L/SIM900 modem ove ## Architecture — which implementation is live -**`GSM-fastapi/` is the deployed, current implementation.** Evidence: `docs/deployment/gsm-module.md`, `docs/getting-started/gsm-module-setup.md`, and `docs/features/sms-gateway/README.md` all reference only `GSM-fastapi/`; `server/app/api/gsm.py` proxies to `http://localhost:8001`, and `GSM-fastapi/main.py` hardcodes `uvicorn.run("api:app", port=8001, ...)` — an exact match. +**`GSM-fastapi/` is the current implementation and intended deployment target.** Evidence: `docs/deployment/gsm-module.md`, `docs/getting-started/gsm-module-setup.md`, and `docs/features/sms-gateway/README.md` all reference only `GSM-fastapi/`; `server/app/api/gsm.py` proxies to `http://localhost:8001`, and `GSM-fastapi/main.py` hardcodes `uvicorn.run("api:app", port=8001, ...)`. **`GSM-API/` is a separate, incomplete rewrite — not deployed, not referenced by any doc or by `server/`.** It has in-memory-only session state (no DB persistence), fire-and-forget SMS sends (no delivery confirmation), and an unsynchronized global (`app/gsm/gsm_runtime.py`'s module-level `ser`) shared across threads with no lock. Default to editing `GSM-fastapi/` for SMS-gateway work; only touch `GSM-API/` if a task explicitly asks for it. -**Known doc/code mismatch:** `docs/features/sms-gateway/design.md` describes a different wire protocol (`SEND:`/`RECV:`/`ACK:`/`ERR:` frames, endpoints `/gsm/send`/`/gsm/status`, tables `sms_outbox`/`sms_inbound`) that matches neither actual Python service nor the Arduino firmware. Trust the code (`GSM-fastapi/protocol.py` + the `.ino` firmware) over that doc — treat `design.md` as aspirational/stale. +**Documentation source of truth:** use `GSM-fastapi/protocol.py` and the Arduino firmware for serial frames. The design document describes the current `SEND_SMS|` protocol and `/sms/send` route. ### Data flow (GSM-fastapi, the live path) **Inbound:** Arduino emits `SMS_RECEIVED||` over serial → `serial_worker.py`'s `SerialWorker._reader_loop` parses it via `protocol.py` → queued → `api.py`'s async `_inbox_drain()` task offloads to a thread pool → `sms_handler.handle_incoming_sms()` (session/target flow, ban/verified checks against MariaDB) → `database.py`'s `notify_app()` POSTs to the main server's `/gsm/inbound` with an `X-GSM-Secret` header. -**Outbound:** caller (main server or admin frontend's `gsm` page) calls `POST /sms/send` on port 8001 → `SerialWorker.send_sms()` enqueues `SEND_SMS||` and blocks on an `Event` (timeout 60s) → `_sender_loop` writes to serial → Arduino replies `SMS_SENT|`/`SMS_FAILED|` → the reader thread resolves the waiting request. +**Outbound:** caller (main server or admin frontend's `gsm` page) calls `POST /sms/send` on port 8001. `SerialWorker.send_sms()` atomically admits it to a bounded FIFO queue or rejects saturation with HTTP 503. One sender registers an in-flight request, writes `SEND_SMS||`, and owns its confirmation timeout. The reader resolves the request from `SMS_SENT|` or `SMS_FAILED|`. Shutdown stops admission and drains unsent work, but preserves the in-flight request's real result. `SerialWorker` runs two dedicated threads (`_reader_loop`, `_sender_loop`) with proper request/response correlation over the async serial stream, and auto-reconnects every 10s on disconnect. @@ -55,7 +55,6 @@ Three layers: Arduino firmware talking AT commands to a SIM800L/SIM900 modem ove - `GSM-fastapi/sapot.db` is a stale, unused artifact (confirmed in `../docs/database/migrations.md`) — real storage is MariaDB via `config.py`'s `DB_PATH`. Never read from or write to `sapot.db`. - `GSM-API/app/gsm/gsm_runtime.py`'s module-level `ser` is a global shared across threads with no lock — if `GSM-API` is ever revived, this is a live race condition, not a style nit. - `GSM-trial-code.ino` is not wire-compatible with either Python service — never point a deployment at it, even for "quick testing." -- Treating `docs/features/sms-gateway/design.md` as accurate — its protocol/endpoint descriptions don't match the real code (see Architecture). - `server/app/api/gsm.py`'s own code comments refer to "GSM-API" as a generic name for **the GSM service it proxies to** (i.e. the live `GSM-fastapi/`, port 8001) — not the literal `GSM-module/GSM-API/` directory documented above as non-deployed. Don't let those comments override the Architecture section above. ## When Modifying This Project diff --git a/docs/deployment/environment-config.md b/docs/deployment/environment-config.md index ca063b24..8e821b78 100644 --- a/docs/deployment/environment-config.md +++ b/docs/deployment/environment-config.md @@ -43,7 +43,8 @@ GSM_SECRET= > **Note:** `GSM-module/` also contains a separate, undocumented `GSM-API/` directory with its own > app code and a committed `.env.example` (`SAPOT_API_URL`, `GSM_SECRET` only). It is not referenced -> by any doc, systemd unit, or setup guide in this repo — `GSM-fastapi/` is the deployed component +> by any doc, systemd unit, or setup guide in this repo. `GSM-fastapi/` is the current implementation +> and intended deployment target. > (see [gsm-module.md](gsm-module.md) and [gsm-module-setup.md](../getting-started/gsm-module-setup.md)). > Not resolved as part of this pass; flagged for a follow-up doc/architecture decision. @@ -58,6 +59,7 @@ GSM_SECRET= | `SAPOT_API_URL` | `http://localhost:8000` | Base URL the GSM module uses to call back into the SAPOT server (`database.py`) — must match wherever the server actually listens | | `GSM_SECRET` | `""` (empty — webhook auth disabled) | Shared secret sent as `X-GSM-Secret` on both directions of the server↔GSM webhook calls (`database.py`). **Must match the server's `GSM_SECRET`** (see above) | | `SMS_BOT_USER_ID` | unset | User ID the GSM module attributes inbound SMS-originated messages to, when the sender can't be resolved to a registered user (`database.py`) | +| `SMS_SEND_QUEUE_MAXSIZE` | `10` | Maximum waiting outbound requests. Must be an integer greater than or equal to `1`; zero, negatives, blanks, and non-integers fail startup. The one in-flight request is excluded, so resident outbound work is this capacity plus one. | ### Recommended production `gsm.env` @@ -71,6 +73,7 @@ LOG_LEVEL=INFO SAPOT_API_URL=https:// GSM_SECRET= SMS_BOT_USER_ID= +SMS_SEND_QUEUE_MAXSIZE=10 ``` --- diff --git a/docs/deployment/gsm-module.md b/docs/deployment/gsm-module.md index f0394c2e..68baaeba 100644 --- a/docs/deployment/gsm-module.md +++ b/docs/deployment/gsm-module.md @@ -45,6 +45,7 @@ actually reachable from outside the container. | Variable | Default | Purpose | |---|---|---| | `SERIAL_PORT` | `/dev/ttyACM0` | USB serial device for the Arduino/GSM modem | +| `SMS_SEND_QUEUE_MAXSIZE` | `10` | Maximum outbound requests waiting behind the one in-flight request. Must be an integer greater than or equal to `1`. | | `SERIAL_BAUD` | `9600` | Serial baud rate | | `DB_PATH` | `mysql+pymysql://sapot:sapot@localhost:3306/sapot_db` | Database connection (hardcoded default — override in production) | | `HOST` | `127.0.0.1` | FastAPI bind host | @@ -62,29 +63,32 @@ The module ships a pre-seeded SQLite development database at `GSM-module/GSM-fas ## Production systemd -Create `/etc/systemd/system/server-GSM-api.service`: - -```ini -[Unit] -Description=SAPOT GSM API -After=network.target - -[Service] -WorkingDirectory=/home/sapot/YLP-software/GSM-module/GSM-fastapi -ExecStart=/home/sapot/YLP-software/GSM-module/GSM-fastapi/venv/bin/python3 main.py -Restart=always -User=sapot -EnvironmentFile=/etc/sapot/gsm.env - -[Install] -WantedBy=multi-user.target -``` +For a first bare-metal host, copy the tracked reference unit `deployment-scripts/server-GSM-api.service` to `/etc/systemd/system/server-GSM-api.service`. Then reload systemd before enabling or starting it. The tracked unit is not installed automatically. ```bash +sudo cp deployment-scripts/server-GSM-api.service /etc/systemd/system/server-GSM-api.service +sudo systemctl daemon-reload sudo systemctl enable server-GSM-api sudo systemctl start server-GSM-api ``` +## Outbound capacity and overload + +The intended deployment accepts 10 waiting outbound requests and one in-flight serial request by default. Configure `SMS_SEND_QUEUE_MAXSIZE` before the first deployment to change the waiting capacity. When the queue is full, `POST /sms/send` returns HTTP 503 with `QUEUE_FULL`; callers should use bounded backoff and must not retry in a tight loop. Saturation does not cancel an accepted in-flight SMS. + +## Queue diagnostics + +| Field | Meaning | +|---|---| +| `outbound_queue_depth` | Accepted requests waiting for the sender | +| `outbound_queue_capacity` | Configured maximum waiting requests | +| `outbound_in_flight` | Whether a request is being written or awaiting modem confirmation | +| `queue_depth` | Existing inbound queue depth, not outbound capacity | + +## Graceful shutdown + +Shutdown first rejects new work, then resolves queued unsent work with `SERVICE_STOPPING`. It does not relabel the in-flight SMS, which remains active until its modem outcome or sender-owned timeout. The tracked systemd unit and both Compose files set a 150-second stop budget: 5 seconds serial write timeout + 120 seconds maximum internal confirmation timeout + two joins of up to 5 seconds = 135 seconds, plus a 15-second service-manager margin. Recalculate this budget whenever those timeout or join constants change. + --- ## Serial port permissions diff --git a/docs/deployment/maintenance.md b/docs/deployment/maintenance.md index 295d601e..33fd8644 100644 --- a/docs/deployment/maintenance.md +++ b/docs/deployment/maintenance.md @@ -28,7 +28,7 @@ Each component owns its own dependency file — there is no repo-wide update mec | `server/` | `requirements.txt` | Schema is Alembic-managed ([ADR 0007](../adr/0007-alembic-for-server-migrations.md)) — a dependency bump that changes SQLModel/SQLAlchemy/DB-driver behavior can shift what autogenerate emits, so re-run `alembic check` and follow [runbooks.md](runbooks.md#applying-schema-migrations-alembic) if it touches schema. Pin `alembic` itself deliberately. | | `mobile-app/sapot-mobile-app/` | `package.json` | Expo SDK bumps need `expo-doctor` (`pnpm run testAll` includes it) — do not hand-edit `pnpm-lock.yaml` | | `admin-frontend/sapot-admin/` | `package.json` | `pnpm run lint && pnpm run build` after any bump — no test script exists in this component | -| `GSM-module/GSM-fastapi/` | `requirements.txt` | No automated tests — verify manually per [gsm-module-setup.md](../getting-started/gsm-module-setup.md) after any bump | +| `GSM-module/GSM-fastapi/` | `requirements.txt` | Run `cd GSM-module/GSM-fastapi && pytest`; serial I/O and database calls are mocked | | Nix flakes (per component) | `flake.lock` | Never hand-edit; only `nix flake update` should touch it | Never bundle a dependency bump with an unrelated feature change — if it breaks something, you want to be able to tell which caused it. diff --git a/docs/deployment/monitoring-logging.md b/docs/deployment/monitoring-logging.md index a8d75e25..72af3ff2 100644 --- a/docs/deployment/monitoring-logging.md +++ b/docs/deployment/monitoring-logging.md @@ -62,6 +62,8 @@ A second background thread (`expire_announcements_loop`) periodically marks anno The GSM module logs to `GSM-module/GSM-fastapi/sapot.log`. Rotate or clear this file periodically in production. +When outbound admission returns `QUEUE_FULL`, the service logs the outbound queue depth and configured capacity at warning level. It must not log the SMS body. Operators should check modem readiness and throughput, allow the queue to drain, and investigate callers before increasing capacity; raising the limit increases waiting time and worker occupancy. + --- ## Health checks diff --git a/docs/features/sms-gateway/design.md b/docs/features/sms-gateway/design.md index 7b204b9a..9c8f560a 100644 --- a/docs/features/sms-gateway/design.md +++ b/docs/features/sms-gateway/design.md @@ -13,7 +13,7 @@ This feature is server-mediated; it has no P2P path. ``` Main Server (FastAPI) ├── POST /gsm/otp/request ──► generate OTP, store in phone_verification - │ └► POST /gsm/send ──────────────────────────────►┐ + │ └► POST /sms/send ──────────────────────────────►┐ │ │ └── POST /gsm/inbound ◄── (webhook with X-GSM-Secret) ◄────────────────────┤ │ @@ -35,11 +35,11 @@ sequenceDiagram participant Ard as Arduino / modem Note over Main,Ard: Outbound SMS - Main->>GSM: POST /gsm/send { phone, message } - GSM->>GSM: encode SEND:: frame - GSM->>Ard: serial write (SEND frame) - Ard-->>GSM: ACK: or ERR:: - GSM->>GSM: mark sms_outbox delivered/failed + Main->>GSM: POST /sms/send { number, body } + GSM->>GSM: create pending sms_log and admit to bounded FIFO queue + GSM->>Ard: SEND_SMS|number|body + Ard-->>GSM: SMS_SENT|number or SMS_FAILED|number|reason + GSM->>GSM: persist one final sms_log result Note over Main,Ard: Inbound SMS Ard->>GSM: serial RECV:: @@ -53,6 +53,10 @@ sequenceDiagram ### `serial_worker.py` +Outbound work uses a bounded FIFO queue. `SMS_SEND_QUEUE_MAXSIZE` limits waiting work only; one request can additionally be in flight. Admission checks lifecycle state, connection, and modem readiness, then calls `put_nowait()` while holding one short-lived lifecycle lock. Full queues return `QUEUE_FULL`; shutdown rejects unsent work with `SERVICE_STOPPING`. + +The sender writes `SEND_SMS||` with a five-second serial write timeout, then owns the modem confirmation timeout. The HTTP caller has a separate wait. Completion clears the in-flight request before signalling it, so late modem events are ignored. During shutdown, the service drains unsent work, preserves the in-flight result, and lets the polling threads stop without a sentinel. + Owns the serial connection lifecycle: - Opens `/dev/ttyACM0` at 9600 baud on startup. @@ -67,69 +71,39 @@ Defines the serial communication protocol between the GSM service and the Arduin ``` Outbound (service → Arduino): - SEND::\n + SEND_SMS||\n Inbound (Arduino → service): - RECV::\n - ACK:\n - ERR::\n + SMS_RECEIVED||\n + SMS_SENT|\n + SMS_FAILED||\n ``` -The protocol layer encodes/decodes these frames and validates that all required fields are present before passing to the handler. +The protocol parser turns valid frames into serial events for the reader thread. The queue and lifecycle state are in memory; `sms_log` is the delivery audit record. ### `sms_handler.py` -Processes both directions: - -**Outbound flow:** - -```python -async def send_sms(phone: str, message: str) -> str: - message_id = uuid4().hex - frame = protocol.encode_send(phone, message, message_id) - serial_worker.serial_send(frame) - db.insert_pending(message_id, phone, message) - return message_id # returned to caller for tracking -``` - -**Inbound flow:** - -```python -def handle_inbound(line: str): - frame = protocol.decode(line) - if frame.type == "RECV": - # POST to main server webhook - requests.post( - f"{MAIN_SERVER_URL}/gsm/inbound", - json={"from": frame.from_number, "body": frame.message_body}, - headers={"X-GSM-Secret": GSM_SECRET}, - timeout=5 - ) - elif frame.type == "ACK": - db.mark_delivered(frame.message_id) - elif frame.type == "ERR": - db.mark_failed(frame.message_id, frame.detail) -``` +Processes inbound messages and determines any reply or forwarded outbound message. The API or internal sender path creates and updates the `sms_log` entry; the serial reader never writes delivery state. ### GSM FastAPI Routes | Method | Path | Description | |--------|------------|----------------------------------------| -| POST | /gsm/send | Accept send request; enqueue via serial | -| GET | /gsm/status| Return service health and serial state | +| POST | /sms/send | Create a message log and enqueue a bounded serial send | +| GET | /status | Return service health and serial state | -`/gsm/send` is called by the main server; it is not exposed to mobile clients directly. +`/sms/send` is called by the main server; it is not exposed to mobile clients directly. -### Storage — `sapot.db` +### Storage -The GSM service uses a local SQLite database for outbox state: +The GSM service stores its relay-owned state in the database configured by `DB_PATH`: | Table | Purpose | |--------------|--------------------------------------------| -| sms_outbox | Pending and delivered outbound messages | -| sms_inbound | Log of received inbound messages | +| sms_log | Inbound and outbound message audit records | +| sms_session | Per-number conversation state | -In production, `DB_PATH` environment variable points to a MariaDB connection string to replace SQLite. +The committed `sapot.db` is stale and is not the deployment datastore. --- @@ -180,7 +154,7 @@ sequenceDiagram User->>Main: POST /gsm/otp/request { phone } Main->>Main: generate 6-digit OTP, bcrypt hash,
upsert phone_verification (expires_at +10min) - Main->>GSM: POST /gsm/send { phone, "Your SAPOT code is..." } + Main->>GSM: POST /sms/send { number, body } GSM->>Ard: serial SEND frame Ard-->>User: SMS delivered @@ -269,7 +243,7 @@ OTP endpoints use Slowapi on the main server: ## Scalability - Designed for low SMS volume (OTPs and occasional fallback messages), not bulk SMS — a single serial-attached modem has a hard throughput ceiling unsuitable for high-volume sending. -- `sms_outbox`/`sms_inbound` grow unboundedly with no documented retention policy; the sqlite-vs-MariaDB storage note in [migrations.md](../../database/migrations.md#gsm-module-database-note) means production data must be in the MariaDB path, not the stale committed `sapot.db`. +- `sms_log` records grow without a documented retention policy. The deployment datastore is the MariaDB path configured through `DB_PATH`, not the stale committed `sapot.db`. ## Acceptance criteria diff --git a/docs/features/sms-gateway/requirements.md b/docs/features/sms-gateway/requirements.md index 298e58d6..b8ae1149 100644 --- a/docs/features/sms-gateway/requirements.md +++ b/docs/features/sms-gateway/requirements.md @@ -22,16 +22,29 @@ The SMS gateway bridges the main server and an Arduino-based GSM module over a s ### FR-SG-01 — Outbound SMS -- `POST /gsm/send` accepts `{ phone: string, message: string }`. -- Requires a valid user or admin JWT (rescuer role for sending arbitrary SMS; system for OTP). -- The GSM API forwards the message to the Arduino over the serial port (`/dev/ttyACM0`). -- The Arduino commands the SIM800L / SIM900 module to send the SMS. -- Response: `{ success: true, message_id: string }` on success; error detail on failure. +- The direct GSM route is `POST /sms/send` with `{ "number": "+639171234567", "body": "message" }`. +- One serial send is in flight at a time. At most `SMS_SEND_QUEUE_MAXSIZE` additional requests wait in FIFO order. +- Admission is non-blocking. Work beyond capacity receives HTTP 503 with `reason: "QUEUE_FULL"`, is logged as failed, and is never written to serial. +- Unsent work rejected during shutdown receives HTTP 503 with `reason: "SERVICE_STOPPING"`. An in-flight request keeps its actual modem result, `TIMEOUT`, or a reason beginning `WRITE_ERROR: `. + +Queue saturation response: + +```json +{ + "detail": { + "message": "Outbound SMS queue is full", + "reason": "QUEUE_FULL", + "msg_id": "" + } +} +``` + +The stopping response has the same shape with `message` set to `SMS service is stopping` and `reason` set to `SERVICE_STOPPING`. These contracts apply to the direct GSM service. The main server currently does not preserve its upstream HTTP status. ### FR-SG-02 — OTP Request - `POST /gsm/otp/request` accepts `{ phone: string, purpose: "verification" | "recovery" }`. -- The main server generates a 6-digit OTP, stores it in `phone_verification` table with a 10-minute TTL, and calls the GSM API `POST /gsm/send` to deliver it. +- The main server generates a 6-digit OTP, stores it in `phone_verification` table with a 10-minute TTL, and calls the GSM API `POST /sms/send` to deliver it. - A phone number may request at most one OTP per 60 seconds (rate-limited by Slowapi). - Response: `{ success: true, expires_in: 600 }`. @@ -76,7 +89,7 @@ The SMS gateway bridges the main server and an Arduino-based GSM module over a s | ID | Requirement | |----------|-----------------------------------------------------------------------------| -| NFR-SG-01 | SMS delivery attempt must complete or fail within 30 seconds | +| NFR-SG-01 | `GET /health` remains responsive during outbound saturation and does not wait on a lock held across serial or other blocking I/O | | NFR-SG-02 | OTP must expire after exactly 10 minutes | | NFR-SG-03 | Inbound webhook must respond within 5 seconds to avoid Arduino timeout | | NFR-SG-04 | Tests must never use a real serial port or SIM module | diff --git a/docs/features/sms-gateway/testing.md b/docs/features/sms-gateway/testing.md index d9e6f54c..e47b533b 100644 --- a/docs/features/sms-gateway/testing.md +++ b/docs/features/sms-gateway/testing.md @@ -2,6 +2,8 @@ ## Strategy +The current implementation has focused automated tests under `GSM-module/GSM-fastapi/tests/`. They mock serial I/O and database calls and must never open a real serial device. + | Layer | Tooling | Scope | |-------------|----------------------------------|--------------------------------------------------------------------| | Unit | pytest | `protocol.py` encode/decode, `sms_handler` outbound/inbound logic | @@ -28,7 +30,7 @@ - **Serial port** — mock `serial_worker.serial_send`; never open a real `/dev/ttyACM0`. Use `unittest.mock.patch`. - **GSM API HTTP calls** — mock `requests.post` in `sms_handler.handle_inbound`; never hit the real main server. -- **Main server → GSM API calls** — mock the GSM API `POST /gsm/send` with `respx` or `responses`; never hit the real GSM service. +- **Main server → GSM API calls** — mock the GSM API `POST /sms/send` with `respx` or `responses`; never hit the real GSM service. - **Database** — use in-memory SQLite for both the GSM service (`sapot.db`) and main server tests. - **Time** — use `freezegun` for OTP expiry assertions. - **GSM_SECRET** — set via `os.environ` in fixture setup; use a fixed test value `"test-gsm-secret-value"`. @@ -38,25 +40,35 @@ ## Test Cases +### Outbound queue and shutdown + +| Scenario | Expected result | +|---|---| +| Capacity is reached | The next request fails immediately with `QUEUE_FULL` and no serial write | +| Shutdown drains waiting work | Unsent requests resolve as `SERVICE_STOPPING`; the in-flight request keeps its actual result | +| Modem event races timeout | Completion occurs once and late events are ignored | +| Serial write stalls | The finite write timeout produces a reason beginning `WRITE_ERROR: ` | +| Saturated service health check | `/health` remains responsive and detailed health reports outbound depth, capacity, and in-flight state | + ### `protocol.py` — Unit | Scenario | Expected result | |----------|-----------------| -| `encode_send("+639171234567", "hello", "msg-1")` | Returns `"SEND:+639171234567:hello:msg-1\n"` | +| `build_send_sms("+639171234567", "hello")` | Returns `"SEND_SMS|+639171234567|hello\n"` | | `decode("RECV:+639171234567:test message\n")` | Returns frame with `type="RECV"`, `from_number="+639171234567"`, `message_body="test message"` | -| `decode("ACK:msg-1\n")` | Returns frame with `type="ACK"`, `message_id="msg-1"` | +| `parse_line("SMS_SENT|+639171234567\n")` | Returns an `SMS_SENT` event | | `decode("ERR:101:module timeout\n")` | Returns frame with `type="ERR"`, `code="101"`, `detail="module timeout"` | | `decode("INVALID\n")` | Raises `ProtocolError` | | Message body containing colon character | Encoded and decoded without truncation | -### GSM Service — `POST /gsm/send` (Integration) +### GSM Service — `POST /sms/send` (Integration) | Scenario | Expected result | |----------|-----------------| | Valid `{ phone, message }` payload | `serial_worker.serial_send` called with correct encoded frame; row inserted in `sms_outbox` with status `pending`; response `{ success: true, message_id }` | | Arduino ACK received via serial | `sms_outbox` row updated to `delivered` | | Arduino ERR received via serial | `sms_outbox` row updated to `failed` with error detail | -| Serial port unavailable at startup | Service returns 503 on `/gsm/send`; error logged | +| Serial port unavailable at startup | Service returns 503 on `/sms/send`; error logged | | Missing `phone` field | Returns 422 | | `message` exceeds 160 characters | Returns 400 or splits into multiple frames depending on config | @@ -103,7 +115,7 @@ |----------|-----------------| | `handle_inbound("RECV:+639171234567:hello\n")` | `requests.post` called to main server `/gsm/inbound` with correct payload and `X-GSM-Secret` header | | Main server webhook call times out | Error logged; no retry in v1; service continues | -| `handle_inbound("ACK:msg-1\n")` | `sms_outbox` row for `msg-1` updated to `delivered` | +| `SMS_SENT|+639171234567` | Resolves the one in-flight request; the waiting sender persists the final result | | `handle_inbound("ERR:101:timeout\n")` | Matching outbox row updated to `failed` | | Serial line that does not parse | `ProtocolError` caught; error logged; service continues | @@ -111,12 +123,16 @@ ## Test File Locations +- `tests/test_config.py`: queue-capacity defaults and invalid settings. +- `tests/test_serial_worker.py`: capacity, lifecycle cutoff, exact-once completion, shutdown draining, and serial write timeout. +- `tests/test_api_queue.py`: 503 queue and shutdown contracts plus saturation diagnostics. + ``` GSM-module/GSM-fastapi/ tests/ - test_protocol.py - test_sms_handler.py - test_gsm_api.py + test_config.py + test_serial_worker.py + test_api_queue.py server/ tests/ From 28bf4944fda6168dad6c116d59a8c67a0c09dd45 Mon Sep 17 00:00:00 2001 From: Adamskiee Date: Tue, 11 Aug 2026 17:19:01 +0800 Subject: [PATCH 04/15] fix(gsm-queue): harden request lifecycle --- GSM-module/GSM-fastapi/config.py | 15 ++-- GSM-module/GSM-fastapi/serial_worker.py | 113 +++++++++++++++--------- 2 files changed, 79 insertions(+), 49 deletions(-) diff --git a/GSM-module/GSM-fastapi/config.py b/GSM-module/GSM-fastapi/config.py index 3d279489..34b64921 100644 --- a/GSM-module/GSM-fastapi/config.py +++ b/GSM-module/GSM-fastapi/config.py @@ -11,9 +11,10 @@ import os from dotenv import load_dotenv +from serial_worker import MAX_SEND_QUEUE_SIZE -def positive_integer_env(name: str, default: int) -> int: +def bounded_integer_env(name: str, default: int, maximum: int) -> int: value = os.environ.get(name) if value is None: return default @@ -21,11 +22,11 @@ def positive_integer_env(name: str, default: int) -> int: parsed = int(value) except ValueError as error: raise RuntimeError( - f"Environment variable '{name}' must be an integer greater than or equal to 1." + f"Environment variable '{name}' must be an integer between 1 and {maximum}." ) from error - if parsed < 1: + if not 1 <= parsed <= maximum: raise RuntimeError( - f"Environment variable '{name}' must be an integer greater than or equal to 1." + f"Environment variable '{name}' must be an integer between 1 and {maximum}." ) return parsed @@ -38,7 +39,6 @@ class Settings: # Baud rate — must match PC_BAUD in the Arduino sketch (9600) serial_baud: int = int(os.environ.get("SERIAL_BAUD", "9600")) - # SQLite database file path db_path: str | None = os.environ.get("DB_PATH") if not db_path: @@ -51,8 +51,9 @@ class Settings: # Logging level log_level: str = os.environ.get("LOG_LEVEL", "INFO") - # Maximum number of outbound SMS requests waiting behind the in-flight send - sms_send_queue_maxsize: int = positive_integer_env("SMS_SEND_QUEUE_MAXSIZE", 10) + sms_send_queue_maxsize: int = bounded_integer_env( + "SMS_SEND_QUEUE_MAXSIZE", 10, MAX_SEND_QUEUE_SIZE + ) settings = Settings() diff --git a/GSM-module/GSM-fastapi/serial_worker.py b/GSM-module/GSM-fastapi/serial_worker.py index 417ac6fe..9b1fcbf8 100644 --- a/GSM-module/GSM-fastapi/serial_worker.py +++ b/GSM-module/GSM-fastapi/serial_worker.py @@ -16,8 +16,8 @@ Incoming SMS events land on worker.incoming_queue as SerialEvent objects. Auto-reconnect: - Serial errors trigger a reconnect loop. Any queued sends are failed - immediately so callers don't hang. The API stays up throughout. + Serial errors fail the active send and trigger a reconnect loop. Waiting + sends remain queued for recovery. The API stays up throughout. """ import logging @@ -34,14 +34,15 @@ logger = logging.getLogger("sapot.serial") RECONNECT_DELAY = 10 # seconds between reconnect attempts +MAX_SEND_QUEUE_SIZE = 20 class OutboundQueueFullError(RuntimeError): - """Raised when no outbound waiting slot is available.""" + pass class WorkerStoppingError(RuntimeError): - """Raised when a request arrives after the shutdown admission cutoff.""" + pass @dataclass @@ -69,6 +70,10 @@ class SerialWorker: def __init__(self, port: str, baud: int = 9600, send_queue_maxsize: int = 10): + if not 1 <= send_queue_maxsize <= MAX_SEND_QUEUE_SIZE: + raise ValueError( + f"send_queue_maxsize must be between 1 and {MAX_SEND_QUEUE_SIZE}" + ) self._port = port self._baud = baud @@ -83,6 +88,9 @@ def __init__(self, port: str, baud: int = 9600, self._send_queue: queue.Queue[_SendRequest] = queue.Queue( maxsize=send_queue_maxsize) + self._active_request: Optional[_SendRequest] = None + self._active_lock = threading.Lock() + # The one request currently being sent (set by sender, read by reader) self._in_flight: Optional[_SendRequest] = None self._in_flight_lock = threading.Lock() @@ -107,25 +115,16 @@ def start(self): self._sender_thread.start() def stop(self): - """Stop admission first, then preserve the current send's true result.""" with self._stop_lock: if self._stop.is_set(): return with self._lifecycle_lock: self._accepting = False + self._stop.set() self._drain_queued_requests() - - # Once admission is closed, a sender can no longer move a dequeued - # request into flight. A request observed here is therefore the only - # one that can still reach the serial port. - with self._in_flight_lock: - in_flight = self._in_flight - if in_flight is not None: - in_flight.done.wait() - - self._stop.set() + self._fail_active_request("SERVICE_STOPPING") self._reader_thread.join(timeout=5) self._sender_thread.join(timeout=5) @@ -197,9 +196,8 @@ def _sender_loop(self): if not self._accepting: self._complete_request(req, False, "SERVICE_STOPPING") continue - # Register before writing so the reader never misses an event. - with self._in_flight_lock: - self._in_flight = req + with self._active_lock: + self._active_request = req # Wait until modem is ready (e.g. after reconnect) deadline = time.time() + req.timeout @@ -207,22 +205,13 @@ def _sender_loop(self): time.sleep(0.5) if not self.gsm_ready: - self._complete_in_flight(req, False, "MODEM_NOT_READY") - logger.warning("SMS to %s dropped — modem not ready", req.number) + self._complete_active_request(req, False, "MODEM_NOT_READY") + logger.warning("SMS to %s dropped: modem not ready", req.number) continue cmd = build_send_sms(req.number, req.body) - try: - with self._ser_lock: - if self._ser and self._ser.is_open: - self._ser.write(cmd.encode("utf-8")) - logger.info("SMS sent to serial: to=%s body=%r", - req.number, req.body) - else: - raise OSError("Serial port not open") - except Exception as e: - logger.error("Serial write failed: %s", e) - self._complete_in_flight(req, False, f"WRITE_ERROR: {e}") + if not self._write_active_request(req, cmd): + self._clear_active_request(req) continue # Block here until the reader resolves this request @@ -231,6 +220,7 @@ def _sender_loop(self): if not resolved: if self._complete_in_flight(req, False, "TIMEOUT"): logger.error("SMS to %s timed out", req.number) + self._clear_active_request(req) # ── Reader thread: serial → events ──────────────────────────────────────── @@ -247,11 +237,11 @@ def _reader_loop(self): self.connected = False self.gsm_ready = False - self.last_status = f"disconnected — retrying in {RECONNECT_DELAY}s" + self.last_status = f"disconnected; retrying in {RECONNECT_DELAY}s" logger.warning("Reconnecting in %ds…", RECONNECT_DELAY) # Fail any in-flight request so the sender doesn't hang - self._fail_in_flight("SERIAL_DISCONNECTED") + self._fail_active_request("SERIAL_DISCONNECTED") time.sleep(RECONNECT_DELAY) @@ -321,15 +311,15 @@ def _handle_line(self, line: str): if etype == EventType.NETWORK_LOST: self.gsm_ready = False self.last_status = "network lost" - logger.warning("Network LOST — in-flight SMS will fail") - self._fail_in_flight("NETWORK_LOST") + logger.warning("Network LOST; in-flight SMS will fail") + self._fail_active_request("NETWORK_LOST") return if etype == EventType.SIM_MISSING: self.gsm_ready = False self.last_status = "SIM missing" logger.error("SIM missing") - self._fail_in_flight("SIM_MISSING") + self._fail_active_request("SIM_MISSING") return if etype == EventType.SMS_SENT: @@ -365,16 +355,55 @@ def _resolve_in_flight(self, number: str, success: bool, logger.warning( "SMS_SENT/FAILED number mismatch: expected %s got %s", req.number, number) - # Still resolve it — the Arduino only handles one at a time + return self._complete_in_flight(req, success, reason) - def _fail_in_flight(self, reason: str): - with self._in_flight_lock: - req = self._in_flight + def _write_active_request(self, req: _SendRequest, cmd: str) -> bool: + # Active state serializes failure with the transition to in-flight. + with self._active_lock: + if self._active_request is not req or req.done.is_set(): + return False + with self._in_flight_lock: + self._in_flight = req + try: + with self._ser_lock: + if not self._ser or not self._ser.is_open: + raise OSError("Serial port not open") + self._ser.write(cmd.encode("utf-8")) + except Exception as error: + logger.error("Serial write failed: %s", error) + self._in_flight = None + self._complete_request(req, False, f"WRITE_ERROR: {error}") + return False + + logger.info("SMS sent to serial: to=%s body=%r", req.number, req.body) + return True + + def _fail_active_request(self, reason: str): + with self._active_lock: + req = self._active_request if req is None: return - if self._complete_in_flight(req, False, reason): - logger.warning("In-flight SMS to %s failed: %s", req.number, reason) + if not req.done.is_set(): + if not self._complete_in_flight(req, False, reason): + self._complete_request(req, False, reason) + logger.warning("Active SMS to %s failed: %s", req.number, reason) + self._active_request = None + + def _complete_active_request(self, req: _SendRequest, success: bool, + reason: Optional[str]) -> bool: + with self._active_lock: + if self._active_request is not req or req.done.is_set(): + return False + if not self._complete_in_flight(req, success, reason): + self._complete_request(req, success, reason) + self._active_request = None + return True + + def _clear_active_request(self, req: _SendRequest): + with self._active_lock: + if self._active_request is req: + self._active_request = None def _complete_in_flight(self, req: _SendRequest, success: bool, reason: Optional[str]) -> bool: From f2689dd6f4f46bcc2be290236990e29f65072fef Mon Sep 17 00:00:00 2001 From: Adamskiee Date: Tue, 11 Aug 2026 17:19:02 +0800 Subject: [PATCH 05/15] test(gsm-queue): cover saturation races --- .../GSM-fastapi/tests/test_api_queue.py | 65 ++++++++++- GSM-module/GSM-fastapi/tests/test_config.py | 21 ++-- .../GSM-fastapi/tests/test_serial_worker.py | 104 ++++++++++++++++++ 3 files changed, 179 insertions(+), 11 deletions(-) diff --git a/GSM-module/GSM-fastapi/tests/test_api_queue.py b/GSM-module/GSM-fastapi/tests/test_api_queue.py index b71b4fc4..94375bb6 100644 --- a/GSM-module/GSM-fastapi/tests/test_api_queue.py +++ b/GSM-module/GSM-fastapi/tests/test_api_queue.py @@ -1,8 +1,16 @@ +import asyncio +import threading + from fastapi.testclient import TestClient +import httpx import pytest import api -from serial_worker import OutboundQueueFullError, WorkerStoppingError +from serial_worker import ( + MAX_SEND_QUEUE_SIZE, + OutboundQueueFullError, + WorkerStoppingError, +) @pytest.fixture(autouse=True) @@ -77,3 +85,58 @@ def __init__(self): assert response.json()["outbound_queue_depth"] == 1 assert response.json()["outbound_queue_capacity"] == 1 assert response.json()["outbound_in_flight"] is True + + +def test_maximum_capacity_still_rejects_and_serves_health(monkeypatch): + class SaturatingWorker: + gsm_ready = True + connected = True + last_status = "ready" + + def __init__(self, capacity): + self.capacity = capacity + self.admitted = 0 + self.lock = threading.Lock() + self.release = threading.Event() + + def send_sms(self, *_args, **_kwargs): + with self.lock: + if self.admitted >= self.capacity: + raise OutboundQueueFullError() + self.admitted += 1 + self.release.wait(timeout=5) + return {"ok": True, "reason": None} + + worker = SaturatingWorker(capacity=MAX_SEND_QUEUE_SIZE + 1) + api._worker = worker + monkeypatch.setattr(api.database, "log_message", lambda **_kwargs: "message-id") + monkeypatch.setattr(api.database, "update_message_status", lambda *_args: None) + + async def exercise_saturation(): + transport = httpx.ASGITransport(app=api.app) + async with httpx.AsyncClient(transport=transport, base_url="http://test") as client: + payload = {"number": "+639171234567", "body": "message"} + admitted = [asyncio.create_task(client.post("/sms/send", json=payload)) + for _ in range(worker.capacity)] + + for _ in range(100): + with worker.lock: + if worker.admitted == worker.capacity: + break + await asyncio.sleep(0.01) + + rejected = await asyncio.wait_for( + client.post("/sms/send", json=payload), timeout=1 + ) + health = await asyncio.wait_for(client.get("/health"), timeout=1) + worker.release.set() + completed = await asyncio.gather(*admitted) + + return rejected, health, completed + + rejected, health, completed = asyncio.run(exercise_saturation()) + + assert rejected.status_code == 503 + assert rejected.json()["detail"]["reason"] == "QUEUE_FULL" + assert health.status_code == 200 + assert all(response.status_code == 200 for response in completed) diff --git a/GSM-module/GSM-fastapi/tests/test_config.py b/GSM-module/GSM-fastapi/tests/test_config.py index 05b9ce2b..c0dd62e4 100644 --- a/GSM-module/GSM-fastapi/tests/test_config.py +++ b/GSM-module/GSM-fastapi/tests/test_config.py @@ -1,23 +1,24 @@ import pytest -from config import positive_integer_env +from config import bounded_integer_env -def test_positive_integer_env_uses_default_when_missing(monkeypatch): +def test_bounded_integer_env_uses_default_when_missing(monkeypatch): monkeypatch.delenv("TEST_QUEUE_SIZE", raising=False) - assert positive_integer_env("TEST_QUEUE_SIZE", 10) == 10 + assert bounded_integer_env("TEST_QUEUE_SIZE", 10, 20) == 10 -@pytest.mark.parametrize("value", ["0", "-1", "", "ten"]) -def test_positive_integer_env_rejects_invalid_values(monkeypatch, value): +@pytest.mark.parametrize("value", ["0", "-1", "", "ten", "21"]) +def test_bounded_integer_env_rejects_invalid_values(monkeypatch, value): monkeypatch.setenv("TEST_QUEUE_SIZE", value) - with pytest.raises(RuntimeError, match="TEST_QUEUE_SIZE.*integer"): - positive_integer_env("TEST_QUEUE_SIZE", 10) + with pytest.raises(RuntimeError, match="TEST_QUEUE_SIZE.*between 1 and 20"): + bounded_integer_env("TEST_QUEUE_SIZE", 10, 20) -def test_positive_integer_env_accepts_positive_value(monkeypatch): - monkeypatch.setenv("TEST_QUEUE_SIZE", "4") +@pytest.mark.parametrize("value", ["1", "4", "20"]) +def test_bounded_integer_env_accepts_value_in_range(monkeypatch, value): + monkeypatch.setenv("TEST_QUEUE_SIZE", value) - assert positive_integer_env("TEST_QUEUE_SIZE", 10) == 4 + assert bounded_integer_env("TEST_QUEUE_SIZE", 10, 20) == int(value) diff --git a/GSM-module/GSM-fastapi/tests/test_serial_worker.py b/GSM-module/GSM-fastapi/tests/test_serial_worker.py index fd04de16..82df4130 100644 --- a/GSM-module/GSM-fastapi/tests/test_serial_worker.py +++ b/GSM-module/GSM-fastapi/tests/test_serial_worker.py @@ -1,3 +1,6 @@ +import threading +import time + import pytest from serial_worker import ( @@ -43,6 +46,7 @@ def test_stop_drains_waiting_requests_without_sentinel(): worker = ready_worker() request = _SendRequest("+639171234567", "queued", 1) worker._send_queue.put_nowait(request) + class JoinedThread: def join(self, timeout): assert timeout == 5 @@ -57,6 +61,26 @@ def join(self, timeout): assert worker.outbound_queue_depth == 0 +def test_stop_fails_active_request(): + worker = ready_worker() + request = _SendRequest("+639171234567", "active", 1) + with worker._active_lock: + worker._active_request = request + + class JoinedThread: + def join(self, timeout): + assert timeout == 5 + + worker._reader_thread = JoinedThread() + worker._sender_thread = JoinedThread() + + worker.stop() + + assert request.done.is_set() + assert request.reason == "SERVICE_STOPPING" + assert worker.outbound_in_flight is False + + def test_admission_rejects_after_shutdown_cutoff(): worker = ready_worker() with worker._lifecycle_lock: @@ -78,6 +102,86 @@ def test_in_flight_completion_is_exact_once(): assert request.reason is None +def test_failure_before_write_prevents_late_serial_send(): + writes = [] + + class FakeSerial: + is_open = True + + def write(self, payload): + writes.append(payload) + + worker = ready_worker() + worker.gsm_ready = False + worker._ser = FakeSerial() + request = _SendRequest("+639171234567", "message", 1) + worker._send_queue.put_nowait(request) + sender = threading.Thread(target=worker._sender_loop) + sender.start() + + for _ in range(100): + with worker._active_lock: + if worker._active_request is request: + break + time.sleep(0.01) + + worker._fail_active_request("NETWORK_LOST") + worker.gsm_ready = True + time.sleep(0.1) + worker._stop.set() + sender.join(timeout=2) + + assert request.done.is_set() + assert request.reason == "NETWORK_LOST" + assert writes == [] + + +def test_stale_confirmation_cannot_complete_request_before_write(): + writes = [] + + class FakeSerial: + is_open = True + + def write(self, payload): + writes.append(payload) + + worker = ready_worker() + worker.gsm_ready = False + worker._ser = FakeSerial() + request = _SendRequest("+639171234568", "message", 1) + worker._send_queue.put_nowait(request) + sender = threading.Thread(target=worker._sender_loop) + sender.start() + + for _ in range(100): + with worker._active_lock: + if worker._active_request is request: + break + time.sleep(0.01) + + worker._resolve_in_flight("+639171234567", True, None) + assert request.done.is_set() is False + + worker.gsm_ready = True + for _ in range(100): + if writes: + break + time.sleep(0.01) + worker._resolve_in_flight(request.number, True, None) + worker._stop.set() + sender.join(timeout=2) + + assert writes == [b"SEND_SMS|+639171234568|message\n"] + assert request.success is True + assert sender.is_alive() is False + + +@pytest.mark.parametrize("capacity", [0, 21]) +def test_constructor_rejects_capacity_outside_threadpool_safe_range(capacity): + with pytest.raises(ValueError, match="between 1 and 20"): + SerialWorker("fake", send_queue_maxsize=capacity) + + def test_serial_connection_uses_finite_write_timeout(monkeypatch): captured = {} From fad3aea373c5fff369e97a00cd7b42535ccad79e Mon Sep 17 00:00:00 2001 From: Adamskiee Date: Tue, 11 Aug 2026 17:19:02 +0800 Subject: [PATCH 06/15] fix(deploy-gsm): align runtime configuration --- GSM-module/GSM-fastapi/.env.example | 4 +- SECURITY.md | 8 +++- deploy/config/gsm-fastapi.env.example | 2 +- deployment-scripts/server-GSM-api.service | 2 +- docker-compose.prod.yml | 1 - docker-compose.yml | 1 - docs/TROUBLESHOOTING.md | 4 +- .../assumptions-and-constraints.md | 1 - docs/architecture/threat-model.md | 1 - docs/deployment/environment-config.md | 10 ++--- docs/deployment/gsm-module.md | 40 +++++++++++-------- docs/deployment/monitoring-logging.md | 13 +++++- docs/getting-started/gsm-module-setup.md | 7 ++-- 13 files changed, 56 insertions(+), 38 deletions(-) diff --git a/GSM-module/GSM-fastapi/.env.example b/GSM-module/GSM-fastapi/.env.example index 3d328628..ebc8eebb 100644 --- a/GSM-module/GSM-fastapi/.env.example +++ b/GSM-module/GSM-fastapi/.env.example @@ -9,8 +9,8 @@ HOST=127.0.0.1 PORT=8000 LOG_LEVEL=INFO # Maximum outbound SMS requests waiting behind the one in-flight request. -# Must be an integer greater than or equal to 1. +# Must be an integer from 1 through 20. SMS_SEND_QUEUE_MAXSIZE=10 -SAPOT_API_URL=https://localhost:8000 +SAPOT_API_URL=http://localhost:8000 GSM_SECRET=change-me-to-a-strong-secret SMS_BOT_USER_ID= diff --git a/SECURITY.md b/SECURITY.md index 6d6dba43..81a0c6a9 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -9,6 +9,7 @@ This is the canonical source of truth for SAPOT's known security-relevant config | Issue | Location | Fix | |---|---|---| | Hardcoded MariaDB credentials | `server/app/db_operations/auth.py` (`SQLALCHEMY_DATABASE_URL`) | Now required via `DATABASE_URL` env var; the app raises `RuntimeError` at import time if unset. **Rotate the previously-hardcoded DB password before deploying this fix.** | +| Hardcoded GSM MariaDB credentials | `GSM-module/GSM-fastapi/config.py` (`db_path`) | Now required via `DB_PATH`; the GSM service raises `RuntimeError` at import time if unset. **Rotate the previously-hardcoded DB password before deploying this fix.** | | Hardcoded JWT secret fallback | `server/app/db_operations/token.py` (`SECRET_KEY`) | The default value has been removed; `JWT_SECRET_KEY` is now required, and the app raises `RuntimeError` at import time if unset. **Rotate to a newly generated secret** (`openssl rand -hex 32`) — the old hardcoded value must be considered compromised since it was committed to source. | | CORS wildcard + credentials | `server/app/main.py` | `allow_origins=["*"]` replaced with an explicit allowlist read from `CORS_ALLOWED_ORIGINS` (comma-separated). The app raises `RuntimeError` at import time if unset. | | Testing router in production | `server/app/main.py` | `app.include_router(testing.router)` is now gated behind `ENVIRONMENT=development` (see `app/main.py`). The `/testing/*` endpoints (`test-make-admin`, `test-make-rescuer`) are unreachable unless the server is explicitly started in development mode. | @@ -23,6 +24,12 @@ JWT_SECRET_KEY= CORS_ALLOWED_ORIGINS=http://192.168.0.100:3000 ``` +Set this in `/etc/sapot/gsm.env` before starting the GSM service: + +```dotenv +DB_PATH=mysql+pymysql://:@127.0.0.1:3306/sapot_db +``` + ## Reporting a vulnerability This is a LAN-deployed application without a public bug bounty program. Report suspected vulnerabilities directly to the maintainer rather than opening a public GitHub issue. @@ -31,7 +38,6 @@ This is a LAN-deployed application without a public bug bounty program. Report s | Gap | Location | Risk | |---|---|---| -| GSM module DB credentials | Hardcoded default in `GSM-module/GSM-fastapi/config.py` (`db_path`) | Same class of risk as the server's DB URL; not yet env-var-only. | | Optional (not enforced) server-side `PeerKey` signing | `SERVER_ED25519_SEED` env var | If unset, a compromised server can MITM new conversations by substituting public keys. See [docs/architecture/threat-model.md](docs/architecture/threat-model.md#e2e-encryption-design-risks). | | No remote session/device revocation UI | Mobile app | A stolen, already-unlocked device has an unbounded access window until an admin manually suspends the account. See [docs/architecture/threat-model.md](docs/architecture/threat-model.md#device-theft). | diff --git a/deploy/config/gsm-fastapi.env.example b/deploy/config/gsm-fastapi.env.example index 8db25e8c..42aecf81 100644 --- a/deploy/config/gsm-fastapi.env.example +++ b/deploy/config/gsm-fastapi.env.example @@ -5,7 +5,7 @@ HOST=0.0.0.0 PORT=8001 LOG_LEVEL=INFO # Maximum outbound SMS requests waiting behind the one in-flight request. -# Must be an integer greater than or equal to 1. +# Must be an integer from 1 through 20. SMS_SEND_QUEUE_MAXSIZE=10 SAPOT_API_URL=https://nginx GSM_SECRET=__FROM_SERVER_GSM_SECRET__ diff --git a/deployment-scripts/server-GSM-api.service b/deployment-scripts/server-GSM-api.service index f0f5f44f..72b17198 100644 --- a/deployment-scripts/server-GSM-api.service +++ b/deployment-scripts/server-GSM-api.service @@ -7,8 +7,8 @@ User=sapot Group=sapot WorkingDirectory=/home/sapot/YLP-software/GSM-module/GSM-fastapi ExecStart=/home/sapot/YLP-software/GSM-module/GSM-fastapi/run-api.sh +EnvironmentFile=/etc/sapot/gsm.env Restart=always -TimeoutStopSec=150 # Automatic restart logic RestartSec=3 diff --git a/docker-compose.prod.yml b/docker-compose.prod.yml index a5748d57..88e8725e 100644 --- a/docker-compose.prod.yml +++ b/docker-compose.prod.yml @@ -70,7 +70,6 @@ services: gsm-fastapi: image: sapot/gsm-fastapi:bundle restart: unless-stopped - stop_grace_period: 150s env_file: [../../../shared/gsm-fastapi.env] environment: HOST: 0.0.0.0 diff --git a/docker-compose.yml b/docker-compose.yml index 196827b0..0eaab2eb 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -121,7 +121,6 @@ services: build: context: ./GSM-module/GSM-fastapi restart: unless-stopped - stop_grace_period: 150s env_file: - ./GSM-module/GSM-fastapi/.env environment: diff --git a/docs/TROUBLESHOOTING.md b/docs/TROUBLESHOOTING.md index ca0052de..aa4f2880 100644 --- a/docs/TROUBLESHOOTING.md +++ b/docs/TROUBLESHOOTING.md @@ -72,9 +72,9 @@ If this fails, check MariaDB is running (`sudo systemctl status mariadb`) and th --- -## GSM module and server can't authenticate each other +## Server rejects GSM inbound callbacks -**Symptom:** SMS send/receive fails; server logs show a rejected `X-GSM-Secret` header, or the GSM module logs show the reverse. +**Symptom:** Inbound SMS forwarding fails and the server logs show a rejected `X-GSM-Secret` header. **Cause:** `GSM_SECRET` differs between the two components' env files. diff --git a/docs/architecture/assumptions-and-constraints.md b/docs/architecture/assumptions-and-constraints.md index ae1845df..77e75c37 100644 --- a/docs/architecture/assumptions-and-constraints.md +++ b/docs/architecture/assumptions-and-constraints.md @@ -59,7 +59,6 @@ Risks the project has consciously decided to carry rather than fix, reproduced f - No LAN segmentation — requires router-level VLAN config not currently documented or automated. - `testing` router reachable when `ENVIRONMENT=development` — accepted; operational discipline (never deploy with this setting) is the control. -- GSM module DB credentials hardcoded default in `config.py` — open, tracked in the repo-root `SECURITY.md`. - No remote session/device revocation UI — open. - Optional (not enforced) server-side `PeerKey` signing — open. diff --git a/docs/architecture/threat-model.md b/docs/architecture/threat-model.md index 74a8aaa6..0dce7105 100644 --- a/docs/architecture/threat-model.md +++ b/docs/architecture/threat-model.md @@ -119,7 +119,6 @@ flowchart TB |---|---| | No LAN segmentation (rescuer/admin/civilian devices share one broadcast domain) | Accepted for now — segmentation requires router-level VLAN config not currently documented or automated. | | `testing` router reachable when `ENVIRONMENT=development` | Accepted — intentionally dev-gated per C1 in the former documentation audit tracker; operational discipline (never deploy with `ENVIRONMENT=development`) is the control, not code. | -| GSM module DB credentials hardcoded default in `config.py` | Open — tracked in the repo-root `SECURITY.md`. | | No remote session/device revocation UI for end users | Open — see [Device theft](#device-theft). | | Optional (not enforced) server-side `PeerKey` signing | Open — see [E2E encryption design risks](#e2e-encryption-design-risks). Recommend making `SERVER_ED25519_SEED` mandatory in production as a follow-up. | diff --git a/docs/deployment/environment-config.md b/docs/deployment/environment-config.md index 8e821b78..c910fccb 100644 --- a/docs/deployment/environment-config.md +++ b/docs/deployment/environment-config.md @@ -15,7 +15,7 @@ All SAPOT components are configured via environment variables. This document lis | `QA_API_TOKEN` | None — required, raises `RuntimeError` at import if unset **when `ENVIRONMENT=development`** | Only relevant in development; the `X-QA-Token` header value `/testing/reset` and `/testing/login-as/{handle}` require | | `REDIS_URL` | `redis://localhost:6379` | Set if Redis is on a non-default host/port | | `SERVER_ED25519_SEED` | `None` (server key signing disabled if unset) | Set to enable server-signed peer keys | -| `GSM_SECRET` | `""` (empty — webhook auth disabled) | Set to a shared secret to authenticate GSM module webhooks | +| `GSM_SECRET` | `""` (empty; inbound callbacks rejected) | Set to a shared secret to authenticate GSM module callbacks | See the repo-root `SECURITY.md` for why `DATABASE_URL`, `JWT_SECRET_KEY`, and `CORS_ALLOWED_ORIGINS` became required. @@ -52,14 +52,14 @@ GSM_SECRET= |---|---|---| | `SERIAL_PORT` | `/dev/ttyACM0` | USB serial device path | | `SERIAL_BAUD` | `9600` | Serial baud rate | -| `DB_PATH` | `mysql+pymysql://sapot:sapot@localhost:3306/sapot_db` (hardcoded default in `config.py`) | MariaDB connection string | +| `DB_PATH` | None; startup raises `RuntimeError` when unset | MariaDB connection string; required | | `HOST` | `127.0.0.1` | FastAPI bind host | | `PORT` | `8000` (code default in `config.py`), but **not actually read** — `GSM-fastapi/main.py` hardcodes `uvicorn.run(..., port=8001, ...)` regardless of this variable. The service always listens on `8001` in practice, which is what avoids colliding with the main SAPOT server on `127.0.0.1:8000` — not the `PORT` variable. | Not a real configuration knob today — see `GSM-module/CLAUDE.md`'s "Common Pitfalls" | | `LOG_LEVEL` | `INFO` | Python logging level (`config.py`) | | `SAPOT_API_URL` | `http://localhost:8000` | Base URL the GSM module uses to call back into the SAPOT server (`database.py`) — must match wherever the server actually listens | -| `GSM_SECRET` | `""` (empty — webhook auth disabled) | Shared secret sent as `X-GSM-Secret` on both directions of the server↔GSM webhook calls (`database.py`). **Must match the server's `GSM_SECRET`** (see above) | +| `GSM_SECRET` | `""` (empty) | Shared secret sent by the GSM module when it calls the server's `/gsm/inbound` route. **Must match the server's `GSM_SECRET`** (see above) | | `SMS_BOT_USER_ID` | unset | User ID the GSM module attributes inbound SMS-originated messages to, when the sender can't be resolved to a registered user (`database.py`) | -| `SMS_SEND_QUEUE_MAXSIZE` | `10` | Maximum waiting outbound requests. Must be an integer greater than or equal to `1`; zero, negatives, blanks, and non-integers fail startup. The one in-flight request is excluded, so resident outbound work is this capacity plus one. | +| `SMS_SEND_QUEUE_MAXSIZE` | `10` | Maximum waiting outbound requests. Integers from `1` through `20` are accepted; other values fail startup. The upper bound leaves capacity in FastAPI's default 40-thread worker pool so overload requests can reach the non-blocking admission check. | ### Recommended production `gsm.env` @@ -70,7 +70,7 @@ DB_PATH=mysql+pymysql://:@127.0.0.1:3306/sapot_db HOST=127.0.0.1 PORT=8001 # harmless to set, but has no real effect — main.py always binds 8001 LOG_LEVEL=INFO -SAPOT_API_URL=https:// +SAPOT_API_URL=http://127.0.0.1:8000 GSM_SECRET= SMS_BOT_USER_ID= SMS_SEND_QUEUE_MAXSIZE=10 diff --git a/docs/deployment/gsm-module.md b/docs/deployment/gsm-module.md index 68baaeba..0aee24a4 100644 --- a/docs/deployment/gsm-module.md +++ b/docs/deployment/gsm-module.md @@ -1,6 +1,6 @@ # GSM Module Deployment -The GSM module (`GSM-module/GSM-fastapi/`) is a FastAPI application that bridges an Arduino-connected SIM800L/SIM900 GSM modem to the SAPOT server's SMS webhook. It exposes SMS send/receive endpoints and forwards inbound SMS to the main server. +The GSM module (`GSM-module/GSM-fastapi/`) is a FastAPI application that bridges an Arduino-connected SIM800L/SIM900 GSM modem to the SAPOT server's SMS webhook. It exposes send, message-history, and health endpoints, then forwards serially received SMS to the main server. --- @@ -19,7 +19,9 @@ cd GSM-module/GSM-fastapi/ python3 -m venv venv source venv/bin/activate pip install -r requirements.txt -SERIAL_PORT=/dev/ttyACM0 python3 main.py +cp .env.example .env +# Edit .env and set DB_PATH, GSM_SECRET, SAPOT_API_URL, and the serial device. +python3 main.py ``` Or use the helper script: @@ -28,15 +30,11 @@ Or use the helper script: bash run-api.sh ``` -`main.py` starts FastAPI on `settings.host` (from `config.py`, default `127.0.0.1`) — **but the port is hardcoded to `8001`** in the `uvicorn.run(...)` call, not read from `settings.port`/`PORT`. Setting `PORT` has no effect on the bound port (it only affects the startup log line, which will report the wrong port — see `GSM-module/CLAUDE.md`'s "Common Pitfalls"). `HOST` is honored via `config.py`. +`main.py` starts FastAPI on `settings.host` (from `config.py`, default `127.0.0.1`), but the port is hardcoded to `8001` in the `uvicorn.run(...)` call. It does not read `settings.port` or `PORT`. Setting `PORT` only changes the startup log line. `HOST` is honored via `config.py`. ### Docker (dev/test alternative) -The root `docker-compose.yml` (see [docker-setup.md](../getting-started/docker-setup.md)) -includes a `gsm-fastapi` service alongside the rest of the stack. It passes through the host's -`/dev/ttyACM0` device, so it only starts successfully on a machine with the modem attached — set -`HOST=0.0.0.0` inside the container (already set in the compose service) so the published port is -actually reachable from outside the container. +The root `docker-compose.yml` (see [docker-setup.md](../getting-started/docker-setup.md)) includes a `gsm-fastapi` service alongside the rest of the stack. The base file does not pass through `/dev/ttyACM0`, which lets development stacks start without GSM hardware. Add `docker-compose.gsm-hardware.yml` when the modem is attached. The Compose service already sets `HOST=0.0.0.0` so its published port is reachable from the host. --- @@ -45,36 +43,44 @@ actually reachable from outside the container. | Variable | Default | Purpose | |---|---|---| | `SERIAL_PORT` | `/dev/ttyACM0` | USB serial device for the Arduino/GSM modem | -| `SMS_SEND_QUEUE_MAXSIZE` | `10` | Maximum outbound requests waiting behind the one in-flight request. Must be an integer greater than or equal to `1`. | +| `SMS_SEND_QUEUE_MAXSIZE` | `10` | Maximum outbound requests waiting behind the one in-flight request. Accepts `1` through `20`. | | `SERIAL_BAUD` | `9600` | Serial baud rate | -| `DB_PATH` | `mysql+pymysql://sapot:sapot@localhost:3306/sapot_db` | Database connection (hardcoded default — override in production) | +| `DB_PATH` | None | Required database connection URL; startup fails when unset | | `HOST` | `127.0.0.1` | FastAPI bind host | -| `PORT` | `8000` in `config.py`, but **not actually used** — `main.py` hardcodes port `8001` regardless of this variable | Documented for completeness only; do not rely on it to change the bound port | +| `PORT` | `8000` in `config.py`, but not used for binding | `main.py` always binds port `8001`; do not rely on this setting | +| `SAPOT_API_URL` | `http://localhost:8000` | Base URL for authenticated inbound callbacks to the main server | +| `GSM_SECRET` | Empty string | Must match the main server value in production | -> **Security note:** `DB_PATH` has a hardcoded default with plaintext credentials. Always set it explicitly in production. See [secrets-management.md](secrets-management.md). +> **Security note:** Set `DB_PATH` and `GSM_SECRET` explicitly before startup. Never deploy the placeholder credentials from `.env.example`. See [secrets-management.md](secrets-management.md). --- ## Database -The module ships a pre-seeded SQLite development database at `GSM-module/GSM-fastapi/sapot.db`. Replace it with an empty database or configure `DB_PATH` to point to the production MariaDB instance before deploying. +The committed `GSM-module/GSM-fastapi/sapot.db` file is stale and unused. Set `DB_PATH` to the production MariaDB instance before deploying. --- ## Production systemd -For a first bare-metal host, copy the tracked reference unit `deployment-scripts/server-GSM-api.service` to `/etc/systemd/system/server-GSM-api.service`. Then reload systemd before enabling or starting it. The tracked unit is not installed automatically. +The tracked unit loads `/etc/sapot/gsm.env` through `EnvironmentFile=`. Provision that restricted file from the committed example before installing the unit. The tracked unit is not installed automatically. ```bash +sudo install -d -m 0700 -o sapot -g sapot /etc/sapot +sudo install -m 0600 -o sapot -g sapot \ + GSM-module/GSM-fastapi/.env.example /etc/sapot/gsm.env +sudoedit /etc/sapot/gsm.env sudo cp deployment-scripts/server-GSM-api.service /etc/systemd/system/server-GSM-api.service sudo systemctl daemon-reload sudo systemctl enable server-GSM-api sudo systemctl start server-GSM-api ``` +This is a manual deployment step. Repository updates do not install the unit or refresh `/etc/sapot/gsm.env`; repeat the copy and restart the service when either artifact changes. + ## Outbound capacity and overload -The intended deployment accepts 10 waiting outbound requests and one in-flight serial request by default. Configure `SMS_SEND_QUEUE_MAXSIZE` before the first deployment to change the waiting capacity. When the queue is full, `POST /sms/send` returns HTTP 503 with `QUEUE_FULL`; callers should use bounded backoff and must not retry in a tight loop. Saturation does not cancel an accepted in-flight SMS. +The intended deployment accepts 10 waiting outbound requests and one active serial request by default. Configure `SMS_SEND_QUEUE_MAXSIZE` from `1` through `20` before startup to change the waiting capacity. The upper bound keeps enough of FastAPI's default 40-thread worker pool available to reject overload. When the queue is full, `POST /sms/send` returns HTTP 503 with `QUEUE_FULL`; callers should use bounded backoff and must not retry in a tight loop. ## Queue diagnostics @@ -85,9 +91,9 @@ The intended deployment accepts 10 waiting outbound requests and one in-flight s | `outbound_in_flight` | Whether a request is being written or awaiting modem confirmation | | `queue_depth` | Existing inbound queue depth, not outbound capacity | -## Graceful shutdown +## Shutdown behavior -Shutdown first rejects new work, then resolves queued unsent work with `SERVICE_STOPPING`. It does not relabel the in-flight SMS, which remains active until its modem outcome or sender-owned timeout. The tracked systemd unit and both Compose files set a 150-second stop budget: 5 seconds serial write timeout + 120 seconds maximum internal confirmation timeout + two joins of up to 5 seconds = 135 seconds, plus a 15-second service-manager margin. Recalculate this budget whenever those timeout or join constants change. +Shutdown closes admission before draining waiting work. Queued and active requests resolve with `SERVICE_STOPPING`, allowing blocked callers to return without extending the service manager's normal stop budget. --- diff --git a/docs/deployment/monitoring-logging.md b/docs/deployment/monitoring-logging.md index 72af3ff2..cc44c9ba 100644 --- a/docs/deployment/monitoring-logging.md +++ b/docs/deployment/monitoring-logging.md @@ -62,13 +62,22 @@ A second background thread (`expire_announcements_loop`) periodically marks anno The GSM module logs to `GSM-module/GSM-fastapi/sapot.log`. Rotate or clear this file periodically in production. -When outbound admission returns `QUEUE_FULL`, the service logs the outbound queue depth and configured capacity at warning level. It must not log the SMS body. Operators should check modem readiness and throughput, allow the queue to drain, and investigate callers before increasing capacity; raising the limit increases waiting time and worker occupancy. +When outbound admission returns `QUEUE_FULL`, the service logs the outbound queue depth and configured capacity at warning level. The saturation warning contains no SMS content. Operators should check modem readiness and throughput, allow the queue to drain, and investigate callers before increasing capacity; raising the limit increases waiting time and worker occupancy. --- ## Health checks -No dedicated health-check endpoints are documented. The Nginx proxy (port 443 → Gunicorn :8000) can be used as a liveness check: +The GSM service exposes dedicated liveness and diagnostic endpoints: + +```bash +curl http://127.0.0.1:8001/health +curl http://127.0.0.1:8001/health/detailed +``` + +The first route remains asynchronous during outbound saturation. The detailed route includes modem state and outbound queue depth, capacity, and in-flight state. + +For the main server, the Nginx proxy (port 443 → Gunicorn :8000) can be used as a liveness check: ```bash curl -k https://localhost/auth/exists?identifier=probe@example.com diff --git a/docs/getting-started/gsm-module-setup.md b/docs/getting-started/gsm-module-setup.md index 331f9183..9d34312c 100644 --- a/docs/getting-started/gsm-module-setup.md +++ b/docs/getting-started/gsm-module-setup.md @@ -41,14 +41,15 @@ cp .env.example .env | Variable | Default | Notes | |---|---|---| -| `DB_PATH` | none, **required** | SQLModel URL for the server's MariaDB, e.g. `mysql+pymysql://sapot:sapot@127.0.0.1:3306/sapot_dev`. `config.py` raises `RuntimeError` at import if unset. (Its source comment says "SQLite"; that is stale; the deployed value is MariaDB.) | -| `GSM_SECRET` | `""` | Must match the server's `GSM_SECRET`. The two components authenticate each other's webhook calls with it via the `X-GSM-Secret` header. Left empty, the server rejects this gateway's calls. | +| `DB_PATH` | none, **required** | SQLModel URL for the server's MariaDB, e.g. `mysql+pymysql://sapot:sapot@127.0.0.1:3306/sapot_dev`. `config.py` raises `RuntimeError` at import if unset. | +| `GSM_SECRET` | `""` | Must match the server's `GSM_SECRET`. The gateway sends it as `X-GSM-Secret` when calling `/gsm/inbound`; the server rejects a missing or mismatched value. | | `SAPOT_API_URL` | `http://localhost:8000` | Base URL of the SAPOT server this gateway forwards inbound SMS to (`POST /gsm/inbound`). The Docker service overrides it to `https://nginx`. | | `SERIAL_PORT` | `/dev/ttyACM0` | Serial device the Arduino is on. `COM3`-style on Windows. | | `SERIAL_BAUD` | `9600` | Must match `PC_BAUD` in the Arduino sketch. | | `HOST` | `127.0.0.1` | Bind address. The Docker service overrides this to `0.0.0.0`. | | `PORT` | `8000` | **Not read.** See [Run](#run) below. | | `LOG_LEVEL` | `INFO` | | +| `SMS_SEND_QUEUE_MAXSIZE` | `10` | Maximum waiting outbound SMS requests. Values from `1` through `20` preserve FastAPI worker capacity; startup fails for other values. | | `SMS_BOT_USER_ID` | unset | UUID of the "SMS Bot" user in the SAPOT database. Inbound SMS is written into the app's conversation/message tables as coming from this user, so create it once on the server and paste the UUID here. | See [environment-config.md](../deployment/environment-config.md) for the cross-component view. @@ -82,6 +83,6 @@ quickest way to confirm the `.env` was picked up. ## Next -- [docker-setup.md](docker-setup.md) — the server must have a matching `GSM_SECRET` set for inbound/outbound SMS to authenticate. +- [docker-setup.md](docker-setup.md): the server must have a matching `GSM_SECRET` for inbound GSM callbacks. - [data-flow.md](../architecture/data-flow.md#sms-fallback) — the end-to-end SMS fallback flow diagram. - [TROUBLESHOOTING.md](../TROUBLESHOOTING.md#gsm-module-and-server-cant-authenticate-each-other): when the two sides reject each other's webhooks. From f818bb06e547e5f9b45ed699d8dfca98cc985ba5 Mon Sep 17 00:00:00 2001 From: Adamskiee Date: Tue, 11 Aug 2026 17:19:02 +0800 Subject: [PATCH 07/15] docs(gsm-gateway): correct service contracts --- GSM-module/CLAUDE.md | 10 +- docs/architecture/data-flow.md | 8 +- docs/features/sms-gateway/design.md | 299 +++++++--------------- docs/features/sms-gateway/requirements.md | 158 +++++++----- 4 files changed, 189 insertions(+), 286 deletions(-) diff --git a/GSM-module/CLAUDE.md b/GSM-module/CLAUDE.md index 9d328a88..28d6d050 100644 --- a/GSM-module/CLAUDE.md +++ b/GSM-module/CLAUDE.md @@ -4,21 +4,21 @@ Instructions for Claude Code working in `GSM-module/` — SAPOT's SMS gateway, b ## Project Overview -Three layers: Arduino firmware talking AT commands to a SIM800L/SIM900 modem over serial; a Python (FastAPI) service on the same machine talking to the Arduino over USB serial; the main `server/` proxying to that Python service over HTTP with a shared secret. There are **two parallel Python implementations** in this directory — they are not both live (see Architecture). +Three layers: Arduino firmware talking AT commands to a SIM800L/SIM900 modem over serial; a Python (FastAPI) service on the same machine talking to the Arduino over USB serial; the main `server/` proxying outbound calls over trusted HTTP and authenticating inbound callbacks with a shared secret. There are **two parallel Python implementations** in this directory; they are not both live (see Architecture). ## Architecture — which implementation is live -**`GSM-fastapi/` is the current implementation and intended deployment target.** Evidence: `docs/deployment/gsm-module.md`, `docs/getting-started/gsm-module-setup.md`, and `docs/features/sms-gateway/README.md` all reference only `GSM-fastapi/`; `server/app/api/gsm.py` proxies to `http://localhost:8001`, and `GSM-fastapi/main.py` hardcodes `uvicorn.run("api:app", port=8001, ...)`. +**`GSM-fastapi/` is the deployed, current implementation.** Evidence: `docs/deployment/gsm-module.md`, `docs/getting-started/gsm-module-setup.md`, and `docs/features/sms-gateway/README.md` all reference only `GSM-fastapi/`; `server/app/api/gsm.py` proxies to `http://localhost:8001`, and `GSM-fastapi/main.py` hardcodes `uvicorn.run("api:app", port=8001, ...)`, an exact match. -**`GSM-API/` is a separate, incomplete rewrite — not deployed, not referenced by any doc or by `server/`.** It has in-memory-only session state (no DB persistence), fire-and-forget SMS sends (no delivery confirmation), and an unsynchronized global (`app/gsm/gsm_runtime.py`'s module-level `ser`) shared across threads with no lock. Default to editing `GSM-fastapi/` for SMS-gateway work; only touch `GSM-API/` if a task explicitly asks for it. +**`GSM-API/` is a separate, incomplete rewrite that is not deployed or referenced by `server/`.** It has in-memory-only session state (no DB persistence), fire-and-forget SMS sends (no delivery confirmation), and an unsynchronized global (`app/gsm/gsm_runtime.py`'s module-level `ser`) shared across threads with no lock. Default to editing `GSM-fastapi/` for SMS-gateway work; only touch `GSM-API/` if a task explicitly asks for it. -**Documentation source of truth:** use `GSM-fastapi/protocol.py` and the Arduino firmware for serial frames. The design document describes the current `SEND_SMS|` protocol and `/sms/send` route. +**Documentation source of truth:** `docs/features/sms-gateway/design.md` describes the deployed HTTP routes, queue, lifecycle, and serial flow. Use `GSM-fastapi/protocol.py` and the Arduino firmware as the authoritative definitions for individual serial frames. ### Data flow (GSM-fastapi, the live path) **Inbound:** Arduino emits `SMS_RECEIVED||` over serial → `serial_worker.py`'s `SerialWorker._reader_loop` parses it via `protocol.py` → queued → `api.py`'s async `_inbox_drain()` task offloads to a thread pool → `sms_handler.handle_incoming_sms()` (session/target flow, ban/verified checks against MariaDB) → `database.py`'s `notify_app()` POSTs to the main server's `/gsm/inbound` with an `X-GSM-Secret` header. -**Outbound:** caller (main server or admin frontend's `gsm` page) calls `POST /sms/send` on port 8001. `SerialWorker.send_sms()` atomically admits it to a bounded FIFO queue or rejects saturation with HTTP 503. One sender registers an in-flight request, writes `SEND_SMS||`, and owns its confirmation timeout. The reader resolves the request from `SMS_SENT|` or `SMS_FAILED|`. Shutdown stops admission and drains unsent work, but preserves the in-flight request's real result. +**Outbound:** the main server calls `POST /sms/send` on port 8001. `SerialWorker.send_sms()` atomically admits the request to a bounded FIFO queue or rejects saturation with HTTP 503. The sender writes `SEND_SMS||`, and the reader resolves the request from `SMS_SENT|` or `SMS_FAILED|`. The admin GSM page reads health and message history through the main server. Shutdown rejects queued and active work with `SERVICE_STOPPING`. `SerialWorker` runs two dedicated threads (`_reader_loop`, `_sender_loop`) with proper request/response correlation over the async serial stream, and auto-reconnects every 10s on disconnect. diff --git a/docs/architecture/data-flow.md b/docs/architecture/data-flow.md index a68d9e97..867c6289 100644 --- a/docs/architecture/data-flow.md +++ b/docs/architecture/data-flow.md @@ -112,8 +112,8 @@ sequenceDiagram participant C as Carrier network participant P as Recipient phone - A->>S: POST /gsm/send-sms (target user has no app presence) - S->>G: POST http://localhost:8001/sms/send (X-GSM-Secret header) + A->>S: POST /gsm/sms/send with JWT + S->>G: POST http://localhost:8001/sms/send G->>M: AT command: send SMS (serial_worker.py) M->>C: SMS PDU C->>P: SMS delivered @@ -122,11 +122,11 @@ sequenceDiagram P->>C: SMS reply C->>M: SMS PDU M->>G: serial_worker reads modem, sms_handler.py parses - G->>S: POST /gsm/inbound-sms (X-GSM-Secret header) + G->>S: POST /gsm/inbound with X-GSM-Secret S->>S: resolve sender/target user, create/append SMS conversation ``` -The server and GSM module authenticate each other with a shared `GSM_SECRET` header (`X-GSM-Secret`), not a user session token — see [environment-config.md](../deployment/environment-config.md). +The main server authenticates user-facing outbound requests with a JSON Web Token (JWT). The direct GSM send endpoint is restricted to the trusted host or Compose network and does not require `X-GSM-Secret`. The shared secret authenticates only the GSM module's inbound callback to `/gsm/inbound`; see [environment-config.md](../deployment/environment-config.md). --- diff --git a/docs/features/sms-gateway/design.md b/docs/features/sms-gateway/design.md index 9c8f560a..811edf7d 100644 --- a/docs/features/sms-gateway/design.md +++ b/docs/features/sms-gateway/design.md @@ -1,253 +1,128 @@ -# SMS Gateway — Design +# SMS Gateway: Design ## Overview -The SMS gateway is a separate FastAPI microservice (`GSM-module/GSM-fastapi/`) that bridges the main server and an Arduino-based GSM module over a serial port. The main server calls the GSM API to send SMS; the Arduino forwards inbound SMS back to the main server via a webhook. +The SMS gateway connects SAPOT to an Arduino-controlled GSM modem. The main server sends HTTP requests to the GSM FastAPI service, which serializes outbound SMS commands over USB. Inbound modem events travel back through the GSM service to the main server. -This feature is server-mediated; it has no P2P path. +The deployed implementation is `GSM-module/GSM-fastapi/`. The parallel `GSM-module/GSM-API/` directory is incomplete and is not part of this design. ---- +## Why is the gateway a separate service? -## Architecture +Serial communication is stateful and permits only one outbound command at a time. Keeping it outside the main server gives one process ownership of the serial port and prevents concurrent HTTP requests from interleaving modem commands. -``` -Main Server (FastAPI) - ├── POST /gsm/otp/request ──► generate OTP, store in phone_verification - │ └► POST /sms/send ──────────────────────────────►┐ - │ │ - └── POST /gsm/inbound ◄── (webhook with X-GSM-Secret) ◄────────────────────┤ - │ - GSM FastAPI Service │ - GSM-module/GSM-fastapi/ - ├── serial_worker.py - ├── sms_handler.py - └── protocol.py - │ serial /dev/ttyACM0 - ▼ - Arduino Uno - SIM800L / SIM900 -``` +The boundary also lets the main server remain available when the modem disconnects. The gateway reports readiness and delivery failures without making GSM hardware a dependency of the main API process. + +## How do the components interact? ```mermaid sequenceDiagram - participant Main as Main Server - participant GSM as GSM FastAPI service - participant Ard as Arduino / modem - - Note over Main,Ard: Outbound SMS - Main->>GSM: POST /sms/send { number, body } - GSM->>GSM: create pending sms_log and admit to bounded FIFO queue - GSM->>Ard: SEND_SMS|number|body - Ard-->>GSM: SMS_SENT|number or SMS_FAILED|number|reason - GSM->>GSM: persist one final sms_log result - - Note over Main,Ard: Inbound SMS - Ard->>GSM: serial RECV:: - GSM->>Main: POST /gsm/inbound { from, body }
header X-GSM-Secret - Main->>Main: verify X-GSM-Secret, process inbound SMS -``` - ---- - -## GSM FastAPI Service — `GSM-module/GSM-fastapi/` - -### `serial_worker.py` - -Outbound work uses a bounded FIFO queue. `SMS_SEND_QUEUE_MAXSIZE` limits waiting work only; one request can additionally be in flight. Admission checks lifecycle state, connection, and modem readiness, then calls `put_nowait()` while holding one short-lived lifecycle lock. Full queues return `QUEUE_FULL`; shutdown rejects unsent work with `SERVICE_STOPPING`. - -The sender writes `SEND_SMS||` with a five-second serial write timeout, then owns the modem confirmation timeout. The HTTP caller has a separate wait. Completion clears the in-flight request before signalling it, so late modem events are ignored. During shutdown, the service drains unsent work, preserves the in-flight result, and lets the polling threads stop without a sentinel. - -Owns the serial connection lifecycle: - -- Opens `/dev/ttyACM0` at 9600 baud on startup. -- Runs a background thread that reads lines from the serial port. -- Forwards inbound lines to `sms_handler.handle_inbound(line)`. -- Exposes `serial_send(command: str)` for outbound AT commands. -- Reconnects automatically if the serial port closes unexpectedly. - -### `protocol.py` - -Defines the serial communication protocol between the GSM service and the Arduino: - -``` -Outbound (service → Arduino): - SEND_SMS||\n - -Inbound (Arduino → service): - SMS_RECEIVED||\n - SMS_SENT|\n - SMS_FAILED||\n + participant Client + participant Main as Main server + participant GSM as GSM FastAPI + participant Arduino + + Client->>Main: POST /gsm/sms/send + Main->>GSM: POST /sms/send {number, body} + GSM->>GSM: log pending and admit to bounded queue + GSM->>Arduino: SEND_SMS|number|body + Arduino-->>GSM: SMS_SENT|number or SMS_FAILED|number|reason + GSM-->>Main: HTTP result + + Arduino->>GSM: SMS_RECEIVED|number|body + GSM->>GSM: apply session and target rules + GSM->>Main: POST /gsm/inbound with X-GSM-Secret ``` -The protocol parser turns valid frames into serial events for the reader thread. The queue and lifecycle state are in memory; `sms_log` is the delivery audit record. - -### `sms_handler.py` - -Processes inbound messages and determines any reply or forwarded outbound message. The API or internal sender path creates and updates the `sms_log` entry; the serial reader never writes delivery state. - -### GSM FastAPI Routes - -| Method | Path | Description | -|--------|------------|----------------------------------------| -| POST | /sms/send | Create a message log and enqueue a bounded serial send | -| GET | /status | Return service health and serial state | +The main server authenticates the user-facing `/gsm/sms/send` route with a JSON Web Token (JWT). The GSM service normally listens on `127.0.0.1:8001` and translates trusted local HTTP calls into serial commands. -`/sms/send` is called by the main server; it is not exposed to mobile clients directly. +## How does outbound admission work? -### Storage +`SerialWorker` owns a bounded first-in, first-out queue. `SMS_SEND_QUEUE_MAXSIZE` configures between 1 and 20 waiting requests, with a default of 10. One additional request may be active in the sender. -The GSM service stores its relay-owned state in the database configured by `DB_PATH`: +Admission uses `put_nowait()` while the lifecycle lock is held. A full queue raises `OutboundQueueFullError`, and `POST /sms/send` returns HTTP 503: -| Table | Purpose | -|--------------|--------------------------------------------| -| sms_log | Inbound and outbound message audit records | -| sms_session | Per-number conversation state | - -The committed `sapot.db` is stale and is not the deployment datastore. - ---- - -## Main Server — OTP Flow - -### `POST /gsm/otp/request` - -```python -otp = generate_otp(6) # cryptographically random 6-digit string -expires_at = datetime.utcnow() + timedelta(minutes=10) -db.upsert(PhoneVerification(phone=phone, otp_hash=bcrypt(otp), expires_at=expires_at, used=False)) -gsm_api.send(phone=phone, message=f"Your SAPOT code is {otp}. Valid for 10 minutes.") +```json +{ + "detail": { + "message": "Outbound SMS queue is full", + "reason": "QUEUE_FULL", + "msg_id": "" + } +} ``` -`phone_verification` table: - -| Column | Type | Notes | -|------------|----------|---------------------------------| -| id | UUID | | -| phone | string | E.164 format | -| otp_hash | string | bcrypt hash; never store raw OTP| -| expires_at | datetime | UTC | -| used | boolean | Set true after successful verify| -| created_at | datetime | | +The upper limit leaves worker threads available for overload responses and other synchronous FastAPI routes. `GET /health` is asynchronous, so liveness remains responsive while admitted sends wait for modem results. -### `POST /gsm/otp/verify` +## How is one SMS sent? -```python -row = db.query(PhoneVerification).filter( - phone=phone, - used=False, - expires_at > datetime.utcnow() -).order_by(created_at.desc()).first() +The sender owns one active request at a time: -if not row or not bcrypt.check(otp, row.otp_hash): - raise HTTPException(401) - -row.used = True -db.commit() -``` - -```mermaid -sequenceDiagram - participant User - participant Main as Main Server - participant GSM as GSM FastAPI service - participant Ard as Arduino / modem - - User->>Main: POST /gsm/otp/request { phone } - Main->>Main: generate 6-digit OTP, bcrypt hash,
upsert phone_verification (expires_at +10min) - Main->>GSM: POST /sms/send { number, body } - GSM->>Ard: serial SEND frame - Ard-->>User: SMS delivered - - User->>Main: POST /gsm/otp/verify { phone, otp } - Main->>Main: lookup unused, unexpired phone_verification row - alt otp matches hash - Main->>Main: mark used = true - Main-->>User: 200 OK - else no match / expired / not found - Main-->>User: 401 - end -``` +1. Dequeue and register the request as active. +2. Wait for modem readiness up to the request timeout. +3. Atomically transition the request to in-flight while writing `SEND_SMS||`. +4. Wait for `SMS_SENT` or `SMS_FAILED` from the reader thread. +5. Complete the matching caller and let the API update `sms_log`. -### `POST /gsm/inbound` (Webhook) +The serial connection has a five-second write timeout. Reader events cannot complete a request before its serial write begins. Queue depth excludes the active request. -```python -@router.post("/gsm/inbound") -async def receive_inbound(request: Request, payload: InboundSmsPayload): - secret = request.headers.get("X-GSM-Secret") - if secret != GSM_SECRET: - raise HTTPException(401) - # process inbound SMS: store, parse commands, etc. -``` +Shutdown closes admission, drains waiting work, and resolves active work with `SERVICE_STOPPING`. This avoids blocking on a sentinel when the bounded queue is full. -`GSM_SECRET` is loaded from the environment at startup; the application refuses to start if it is not set. +## What is the serial protocol? ---- +`GSM-fastapi/protocol.py` and the production Arduino firmware are the sources of truth. -## Webhook Authentication +```text +Python to Arduino: + SEND_SMS||\n +Arduino to Python: + GSM_READY + NETWORK_OK + NETWORK_LOST + SIM_MISSING + SMS_RECEIVED||\n + SMS_SENT|\n + SMS_FAILED||\n + LOG|\n ``` -GSM FastAPI service ──POST /gsm/inbound──► Main Server - X-GSM-Secret: -``` - -- `GSM_SECRET` is a shared secret set as an environment variable on both services. -- The main server rejects any `/gsm/inbound` request where the header is absent or does not match. -- In production this secret should be at least 32 random bytes, base64-encoded. - ---- - -## Rate Limiting - -OTP endpoints use Slowapi on the main server: - -| Endpoint | Limit | -|-----------------------|-------------------| -| `/gsm/otp/request` | 1 per 60 s per IP | -| `/gsm/otp/resend` | 1 per 60 s per IP | -| `/gsm/otp/verify` | 5 per 60 s per IP | - ---- -## Dependencies +Message bodies may contain pipe characters. `parse_line()` preserves them for `SMS_RECEIVED`. Newlines in outbound bodies are replaced with spaces by `build_send_sms()`. -| Component | Purpose | -|------------------------|-----------------------------------------------| -| pyserial | Serial port communication with Arduino | -| FastAPI (GSM service) | HTTP API for send/status endpoints | -| SQLite / MariaDB | GSM service outbox state | -| Slowapi | Rate limiting on OTP endpoints (main server) | -| bcrypt | OTP hashing in `phone_verification` | +## How are inbound messages handled? ---- +The reader places `SMS_RECEIVED` events on `incoming_queue`. The API lifespan task passes each event to `handle_incoming_sms()`, which applies registration, ban, phone-verification, session, and target checks. -## Non-goals +The handler can return a reply to the sender and a forwarded message for the selected target. Both use the same bounded outbound queue. `database.notify_app()` also calls the main server's `POST /gsm/inbound` route with `X-GSM-Secret`. -- Not a two-way in-app messaging replacement — SMS is a fallback for OTP delivery and reaching users without the app installed, not a full-featured SMS inbox/thread UI. -- No multi-modem/multi-line support — the current design assumes a single serial-attached modem (`serial_worker.py` owns one connection); sending to multiple numbers concurrently is serialized through that one channel. -- No delivery-status UI beyond `db.mark_delivered`/`db.mark_failed` — there is no user-facing "message delivered/read" indicator for SMS, unlike in-app messages. -- Not encrypted — SMS content is plaintext by the nature of the SMS protocol; see [messaging design's SMS fallback note](../messaging/design.md#sms-fallback). +## What is persisted? -## Failure handling +The GSM service uses the database configured by required `DB_PATH`. -- **Serial port closes unexpectedly:** `serial_worker.py` reconnects automatically; any AT command in flight when the port closes is presumed lost — `sms_handler.py`'s outbox (`sms_outbox` table, `pending`/`delivered`/`failed` states) is the source of truth for what still needs resending, but automatic resend of `pending` rows after a reconnect is not described in the current design — worth confirming as a follow-up. -- **`ERR` frame from the Arduino:** `handle_inbound` marks the corresponding outbox row `failed` with the modem's detail code — the main server's OTP flow surfaces this as an OTP-send failure rather than silently leaving the user waiting. -- **Webhook call to the main server fails** (network blip, main server down): `requests.post(...)` to `/gsm/inbound` has a 5s timeout; a failed webhook call means an inbound SMS is acknowledged to the modem but never reaches the main server — there is no retry/dead-letter queue for this today. -- **`GSM_SECRET` mismatch or missing:** the main server rejects the webhook with 401; the GSM service has no visibility into *why* it was rejected beyond the HTTP status. -- **GSM module fully unreachable from the main server:** phone OTP requests fail; per [account-recovery design](../account-recovery/design.md#failure-handling), other recovery/verification methods remain usable. +| Table | Responsibility | +|---|---| +| `sms_log` | Inbound and outbound audit rows, delivery status, and failure reason | +| `sms_session` | Per-phone conversation stage and selected target | +| Shared user and conversation tables | Lookup and delivery integration with the main server | -## Performance impact +The committed `sapot.db` file is stale and is not used by the deployed service. -- SMS delivery latency is bounded by the modem's own network round-trip (cellular network, typically seconds) — orders of magnitude slower than in-app message delivery; the OTP flow's 10-minute expiry window is sized to tolerate this. -- The serial channel is a single sequential bottleneck — `serial_send` calls queue behind whatever is currently in flight on `/dev/ttyACM0`, so send throughput is capped by modem + serial round-trip time, not by the FastAPI service itself. +## How are failures reported? -## Scalability +| Failure | Result | +|---|---| +| Queue at capacity | HTTP 503 with `QUEUE_FULL`; no serial write | +| Worker stopping | HTTP 503 with `SERVICE_STOPPING` | +| Serial port or modem unavailable before admission | HTTP 503 | +| Serial write error | HTTP 502 with a reason beginning `WRITE_ERROR:` | +| Modem reports failure or confirmation times out | HTTP 502 with the modem or timeout reason | +| Main server cannot reach the GSM service | Main server health route returns HTTP 503 | -- Designed for low SMS volume (OTPs and occasional fallback messages), not bulk SMS — a single serial-attached modem has a hard throughput ceiling unsuitable for high-volume sending. -- `sms_log` records grow without a documented retention policy. The deployment datastore is the MariaDB path configured through `DB_PATH`, not the stale committed `sapot.db`. +The main server currently returns the GSM response body without preserving the upstream status from `POST /sms/send`. Callers using the main server route cannot rely on the gateway's 503 status until that proxy behavior is corrected. -## Acceptance criteria +## Security and deployment assumptions -- An OTP requested via `/gsm/otp/request` is delivered as an SMS and successfully verified via `/gsm/otp/verify` within its 10-minute validity window. -- An unauthenticated (`X-GSM-Secret` mismatch or missing) call to `/gsm/inbound` is rejected with 401 and has no side effects. -- A failed outbound send is reflected in `sms_outbox` as `failed`, not left indefinitely `pending`. -- OTP endpoints enforce their documented rate limits (`1 per 60s` for request/resend, `5 per 60s` for verify). +- Set `DB_PATH` and `GSM_SECRET` in restricted environment files. Bare-metal systemd deployments use `/etc/sapot/gsm.env`. +- The main server checks `X-GSM-Secret` on inbound callbacks. +- The direct GSM service does not authenticate `/sms/send`; keep port 8001 restricted to the host or trusted Compose network. +- SMS content is plaintext on the carrier network and should not be treated as end-to-end encrypted. +- The design supports one serial modem. Multi-modem failover and bulk SMS are out of scope. diff --git a/docs/features/sms-gateway/requirements.md b/docs/features/sms-gateway/requirements.md index b8ae1149..8046eb47 100644 --- a/docs/features/sms-gateway/requirements.md +++ b/docs/features/sms-gateway/requirements.md @@ -1,33 +1,55 @@ -# SMS Gateway — Requirements +# SMS Gateway: Requirements ## Overview -The SMS gateway bridges the main server and an Arduino-based GSM module over a serial connection, letting the server send and receive SMS for OTP delivery and rescuer-initiated outreach to users without app connectivity. +The SMS gateway must let SAPOT send and receive SMS through one serial-attached modem without allowing HTTP load to create an unbounded in-memory backlog. ---- +These requirements describe the deployed `GSM-module/GSM-fastapi/` service and its HTTP integration with the main server. -## User Stories +## User outcomes -| ID | As a… | I want to… | So that… | -|--------|----------|---------------------------------------------------------|-----------------------------------------------------------------| -| SG-01 | rescuer | send an SMS to a registered user's phone number | I can reach them even if they are not connected to the LAN | -| SG-02 | user | receive a one-time password via SMS | I can verify my phone number or recover my account | -| SG-03 | user | resend an OTP if I did not receive it | I am not locked out due to delivery failure | -| SG-04 | system | receive inbound SMS from the GSM module | Users can send text commands or replies back to the system | -| SG-05 | admin | see whether the GSM module is online | I can confirm SMS delivery capability before relying on it | +| ID | User | Outcome | +|---|---|---| +| SG-01 | Rescuer | Send an SMS to a registered phone number | +| SG-02 | User | Receive phone-verification and recovery codes | +| SG-03 | User | Send an SMS reply through the gateway | +| SG-04 | Administrator | Observe modem readiness and queue saturation | ---- +## Functional requirements -## Functional Requirements +### FR-SG-01: Direct outbound SMS -### FR-SG-01 — Outbound SMS +`POST /sms/send` accepts: -- The direct GSM route is `POST /sms/send` with `{ "number": "+639171234567", "body": "message" }`. -- One serial send is in flight at a time. At most `SMS_SEND_QUEUE_MAXSIZE` additional requests wait in FIFO order. -- Admission is non-blocking. Work beyond capacity receives HTTP 503 with `reason: "QUEUE_FULL"`, is logged as failed, and is never written to serial. -- Unsent work rejected during shutdown receives HTTP 503 with `reason: "SERVICE_STOPPING"`. An in-flight request keeps its actual modem result, `TIMEOUT`, or a reason beginning `WRITE_ERROR: `. +```json +{ + "number": "+639171234567", + "body": "message" +} +``` + +The number must use E.164 format. The submitted body must not exceed 160 characters and must remain nonempty after trimming. + +A successful modem confirmation returns HTTP 200: + +```json +{ + "ok": true, + "msg_id": "", + "to": "+639171234567" +} +``` -Queue saturation response: +A modem failure, write failure, or confirmation timeout returns HTTP 502 and records the failure in `sms_log`. + +### FR-SG-02: Bounded outbound admission + +- One outbound request may be active in the serial sender. +- `SMS_SEND_QUEUE_MAXSIZE` permits 1 through 20 additional waiting requests and defaults to 10. +- Waiting requests retain first-in, first-out order. +- Admission must not block when the queue is full. +- Work beyond capacity must never reach the serial port. +- Saturated requests must be logged as failed and return HTTP 503 with `reason: "QUEUE_FULL"`. ```json { @@ -39,67 +61,73 @@ Queue saturation response: } ``` -The stopping response has the same shape with `message` set to `SMS service is stopping` and `reason` set to `SERVICE_STOPPING`. These contracts apply to the direct GSM service. The main server currently does not preserve its upstream HTTP status. +### FR-SG-03: Lifecycle cutoff -### FR-SG-02 — OTP Request +`SerialWorker.stop()` must close admission atomically. Waiting and active requests must resolve with `SERVICE_STOPPING`, and a new request after the cutoff must receive HTTP 503. -- `POST /gsm/otp/request` accepts `{ phone: string, purpose: "verification" | "recovery" }`. -- The main server generates a 6-digit OTP, stores it in `phone_verification` table with a 10-minute TTL, and calls the GSM API `POST /sms/send` to deliver it. -- A phone number may request at most one OTP per 60 seconds (rate-limited by Slowapi). -- Response: `{ success: true, expires_in: 600 }`. +A request completed before the cutoff must not be overwritten. A request failed before its serial write must never be written after modem recovery. -### FR-SG-03 — OTP Verify +### FR-SG-04: Health and diagnostics -- `POST /gsm/otp/verify` accepts `{ phone: string, otp: string }`. -- Looks up the most recent non-expired `phone_verification` row for the phone number. -- Returns 200 `{ verified: true }` if the OTP matches and has not expired. -- Returns 401 if the OTP is wrong. -- Returns 401 if the OTP has expired. -- Marks the row as used after a successful verification (prevents replay). +- `GET /health` must remain responsive while synchronous send requests occupy worker threads. +- `GET /health` returns modem readiness and serial connection state. +- `GET /health/detailed` reports inbound queue depth, outbound waiting depth, outbound capacity, and whether a serial request is in flight. +- Queue saturation must log depth and capacity without including SMS content in the saturation warning. -### FR-SG-04 — OTP Resend +### FR-SG-05: Serial protocol -- `POST /gsm/otp/resend` accepts `{ phone: string }`. -- Invalidates any existing OTP for that phone and generates a new one. -- Subject to the same 60-second rate limit as `/gsm/otp/request`. +The service must send and receive these frames: -### FR-SG-05 — Inbound SMS +```text +SEND_SMS|| +SMS_RECEIVED|| +SMS_SENT| +SMS_FAILED|| +GSM_READY +NETWORK_OK +NETWORK_LOST +SIM_MISSING +``` -- The Arduino receives inbound SMS from the SIM module and forwards it over serial to the GSM FastAPI service. -- The GSM FastAPI service POSTs the SMS content to the main server webhook: `POST /gsm/inbound`. -- The webhook is authenticated with a shared `GSM_SECRET` environment variable (sent as `X-GSM-Secret` header). -- A request without the correct `GSM_SECRET` is rejected with 401. -- The main server processes the inbound SMS content (e.g. parse OTP replies, store message). +Only one request may await a modem confirmation. A confirmation received before a new request starts its serial write must not complete that request. -### FR-SG-06 — Hardware +### FR-SG-06: Inbound SMS -- GSM module: Arduino Uno (or compatible) connected to a SIM800L or SIM900 GSM shield. -- Serial interface: `/dev/ttyACM0` at 9600 baud. -- The GSM FastAPI service runs as a separate systemd unit: `server-GSM-api.service`. -- The service is co-located on the same server host as the main FastAPI application. +- The serial reader must enqueue `SMS_RECEIVED` events for application processing. +- `handle_incoming_sms()` must apply the registered-user, banned-user, verified-phone, session, and target rules. +- The GSM service must call the main server's `POST /gsm/inbound` route with `X-GSM-Secret` when forwarding into the app. +- Failed callbacks are logged. Automatic callback retry is not required. -### FR-SG-07 — GSM Module Storage +### FR-SG-07: Configuration and storage -- The GSM FastAPI service uses a local SQLite database (`sapot.db`) for state (pending outbox, delivery receipts). -- In production this is replaced by a MariaDB connection configured via the `DB_PATH` environment variable. +- `DB_PATH` is required. Startup must raise `RuntimeError` when it is missing. +- Invalid `SMS_SEND_QUEUE_MAXSIZE` values must fail startup. +- `sms_log` stores inbound and outbound audit records. +- `sms_session` stores per-phone relay state. +- The committed `sapot.db` file must not be used as the deployment datastore. ---- +### FR-SG-08: Main server integration -## Non-Functional Requirements +- The user-facing `/gsm/sms/send` route remains on the main server and requires its normal JWT authentication. +- The main server calls the direct gateway at `http://localhost:8001/sms/send`. +- The direct gateway is a trusted local service and must not be exposed to untrusted networks. +- Preserving the gateway's HTTP status through the main server proxy is a known limitation, not part of this change. -| ID | Requirement | -|----------|-----------------------------------------------------------------------------| -| NFR-SG-01 | `GET /health` remains responsive during outbound saturation and does not wait on a lock held across serial or other blocking I/O | -| NFR-SG-02 | OTP must expire after exactly 10 minutes | -| NFR-SG-03 | Inbound webhook must respond within 5 seconds to avoid Arduino timeout | -| NFR-SG-04 | Tests must never use a real serial port or SIM module | -| NFR-SG-05 | `GSM_SECRET` must be set via environment variable; never hardcoded | +## Non-functional requirements ---- +| ID | Requirement | +|---|---| +| NFR-SG-01 | Memory used by outbound waiting work is bounded by the configured queue | +| NFR-SG-02 | The default 40-thread FastAPI worker pool retains headroom at maximum queue capacity | +| NFR-SG-03 | `GET /health` does not wait on serial I/O or synchronous endpoint capacity | +| NFR-SG-04 | Serial writes time out after five seconds | +| NFR-SG-05 | Automated tests never open a real serial device or contact a real modem | +| NFR-SG-06 | Production secrets are supplied through restricted environment files | -## Out of Scope +## Out of scope -- MMS support. -- SMS delivery receipts from the carrier network. -- Multi-SIM failover. -- Direct SIM module management via the admin UI (v1 is send/receive only). +- Bulk SMS and multi-modem scheduling +- Multimedia Messaging Service (MMS) +- Carrier delivery or read receipts +- Automatic retry of failed main-server callbacks +- Preserving direct-gateway HTTP status through the current main-server proxy From 7af393abd1bc5a985a409fb94538427866c4fecd Mon Sep 17 00:00:00 2001 From: Adamskiee Date: Tue, 11 Aug 2026 17:19:02 +0800 Subject: [PATCH 08/15] docs(gsm-testing): document queue verification --- docs/features/sms-gateway/testing.md | 181 +++++++++------------------ 1 file changed, 60 insertions(+), 121 deletions(-) diff --git a/docs/features/sms-gateway/testing.md b/docs/features/sms-gateway/testing.md index e47b533b..ae061a17 100644 --- a/docs/features/sms-gateway/testing.md +++ b/docs/features/sms-gateway/testing.md @@ -1,152 +1,91 @@ -# SMS Gateway — Testing +# SMS Gateway: Testing -## Strategy +## Overview -The current implementation has focused automated tests under `GSM-module/GSM-fastapi/tests/`. They mock serial I/O and database calls and must never open a real serial device. +The GSM FastAPI tests verify queue admission, lifecycle races, API responses, and configuration without opening a real serial port or connecting to the production database. Hardware behavior still requires a modem smoke test because unit tests cannot prove Arduino timing or carrier delivery. -| Layer | Tooling | Scope | -|-------------|----------------------------------|--------------------------------------------------------------------| -| Unit | pytest | `protocol.py` encode/decode, `sms_handler` outbound/inbound logic | -| Integration | pytest + HTTPX + in-memory SQLite| GSM FastAPI endpoints, main server OTP endpoints, webhook handler | -| Hardware | **Never tested against real HW** | Serial port and SIM module are always mocked | +## Prerequisites ---- +Use the component's pinned Nix environment and installed virtual environment: -## Coverage Targets - -| Area | Target | -|-----------------------------------|--------| -| `protocol.py` encode/decode | 100% | -| `sms_handler` outbound path | 100% | -| `sms_handler` inbound path | 100% | -| OTP request / verify / resend | 100% | -| Webhook authentication | 100% | -| Rate limiting enforcement | 90%+ | -| Overall SMS gateway coverage | ≥ 80% | - ---- - -## Mocking Rules +```bash +cd GSM-module/GSM-fastapi +nix develop +pytest +``` -- **Serial port** — mock `serial_worker.serial_send`; never open a real `/dev/ttyACM0`. Use `unittest.mock.patch`. -- **GSM API HTTP calls** — mock `requests.post` in `sms_handler.handle_inbound`; never hit the real main server. -- **Main server → GSM API calls** — mock the GSM API `POST /sms/send` with `respx` or `responses`; never hit the real GSM service. -- **Database** — use in-memory SQLite for both the GSM service (`sapot.db`) and main server tests. -- **Time** — use `freezegun` for OTP expiry assertions. -- **GSM_SECRET** — set via `os.environ` in fixture setup; use a fixed test value `"test-gsm-secret-value"`. -- **OTP generation** — mock `generate_otp` to return a fixed value (`"123456"`) in integration tests for predictable assertions. +When Nix is unavailable, the exact pinned `requirements.txt` can be exercised in an isolated environment: ---- +```bash +uv run --isolated --with-requirements requirements.txt pytest +``` -## Test Cases +`tests/conftest.py` supplies a test `DB_PATH` before importing application settings. Tests that reach API handlers replace database operations with fakes, so they do not create or mutate production records. -### Outbound queue and shutdown +## What is covered? -| Scenario | Expected result | +| File | Responsibility | |---|---| -| Capacity is reached | The next request fails immediately with `QUEUE_FULL` and no serial write | -| Shutdown drains waiting work | Unsent requests resolve as `SERVICE_STOPPING`; the in-flight request keeps its actual result | -| Modem event races timeout | Completion occurs once and late events are ignored | -| Serial write stalls | The finite write timeout produces a reason beginning `WRITE_ERROR: ` | -| Saturated service health check | `/health` remains responsive and detailed health reports outbound depth, capacity, and in-flight state | +| `tests/test_config.py` | Default queue capacity, valid range, and startup rejection | +| `tests/test_serial_worker.py` | Queue capacity, lifecycle cutoff, exact-once completion, pre-write races, and serial write timeout | +| `tests/test_api_queue.py` | HTTP 503 contracts, message-log updates, diagnostics, worker-pool headroom, and health responsiveness | -### `protocol.py` — Unit +The suite currently contains 24 focused tests after the outbound-queue change. -| Scenario | Expected result | -|----------|-----------------| -| `build_send_sms("+639171234567", "hello")` | Returns `"SEND_SMS|+639171234567|hello\n"` | -| `decode("RECV:+639171234567:test message\n")` | Returns frame with `type="RECV"`, `from_number="+639171234567"`, `message_body="test message"` | -| `parse_line("SMS_SENT|+639171234567\n")` | Returns an `SMS_SENT` event | -| `decode("ERR:101:module timeout\n")` | Returns frame with `type="ERR"`, `code="101"`, `detail="module timeout"` | -| `decode("INVALID\n")` | Raises `ProtocolError` | -| Message body containing colon character | Encoded and decoded without truncation | +## How is serial I/O isolated? -### GSM Service — `POST /sms/send` (Integration) +Most worker tests do not start the reader or sender threads. Tests that need a serial write assign a small fake object to `worker._ser`. The connection test replaces `serial_worker.serial.Serial` before calling `_connect_and_read()`. -| Scenario | Expected result | -|----------|-----------------| -| Valid `{ phone, message }` payload | `serial_worker.serial_send` called with correct encoded frame; row inserted in `sms_outbox` with status `pending`; response `{ success: true, message_id }` | -| Arduino ACK received via serial | `sms_outbox` row updated to `delivered` | -| Arduino ERR received via serial | `sms_outbox` row updated to `failed` with error detail | -| Serial port unavailable at startup | Service returns 503 on `/sms/send`; error logged | -| Missing `phone` field | Returns 422 | -| `message` exceeds 160 characters | Returns 400 or splits into multiple frames depending on config | +No test may rely on `/dev/ttyACM0`, a SIM card, or a carrier network. A test that starts a thread must signal it to stop and join it before returning. -### Main Server — OTP Request (Integration) +## Queue and lifecycle cases | Scenario | Expected result | -|----------|-----------------| -| `POST /gsm/otp/request` with valid phone, purpose `"verification"` | `phone_verification` row created with hashed OTP; `expires_at = now + 10 min`; GSM API send called | -| Second request within 60 s for same phone | Returns 429 (rate limit) | -| Request after 60 s | New OTP generated; old row marked superseded | -| GSM API send fails | Returns 503; `phone_verification` row not created | -| Missing `phone` field | Returns 422 | - -### Main Server — OTP Verify (Integration) - -| Scenario | Expected result | -|----------|-----------------| -| Correct OTP within expiry window | Returns 200 `{ verified: true }`; `phone_verification.used` set to `true` | -| Correct OTP reused after first verify | Returns 401 (row is marked `used`) | -| Wrong OTP | Returns 401 | -| OTP expired (`expires_at` in past) | Returns 401 | -| No OTP row exists for phone | Returns 401 | -| `POST /gsm/otp/verify` 6 times in 60 s | 6th request returns 429 (rate limit) | +|---|---| +| Waiting queue reaches configured capacity | Next admission raises `OutboundQueueFullError` without a serial write | +| Capacity is outside 1 through 20 | Configuration or worker construction fails | +| Shutdown begins with waiting work | Waiting requests complete with `SERVICE_STOPPING` | +| Shutdown begins with active work | Active request completes with `SERVICE_STOPPING` | +| Network loss completes active work before writing | Recovery does not write the failed request | +| Stale confirmation arrives before a new write | New request remains pending and is written normally | +| Two completions race for one request | First completion wins | +| Serial connection opens | PySerial receives the configured five-second write timeout | -### Main Server — OTP Resend (Integration) +## API saturation cases -| Scenario | Expected result | -|----------|-----------------| -| `POST /gsm/otp/resend` for phone with existing OTP | Old row invalidated; new `phone_verification` row created; GSM API send called with new OTP | -| Resend within 60 s of last request | Returns 429 | +The saturation test starts 21 blocking send requests, representing 20 waiting requests plus one active request. It then verifies that: -### Inbound Webhook — Main Server (Integration) +1. A further `POST /sms/send` reaches the handler and returns HTTP 503 with `QUEUE_FULL`. +2. `GET /health` returns while those sends remain blocked. +3. Releasing the admitted requests lets every pending HTTP request complete. -| Scenario | Expected result | -|----------|-----------------| -| `POST /gsm/inbound` with correct `X-GSM-Secret` | Returns 200; inbound SMS processed | -| `POST /gsm/inbound` with wrong secret | Returns 401; SMS not processed | -| `POST /gsm/inbound` with missing `X-GSM-Secret` header | Returns 401 | -| `POST /gsm/inbound` with valid secret and OTP-reply body | OTP reply matched to pending verification; user notified | +This covers the thread-pool exhaustion described by issue #252. A test with only a rejecting fake would verify the response shape but would not prove that the rejection handler can still obtain a worker thread. -### GSM Service — Inbound SMS Path (Unit) +## Manual modem smoke test -| Scenario | Expected result | -|----------|-----------------| -| `handle_inbound("RECV:+639171234567:hello\n")` | `requests.post` called to main server `/gsm/inbound` with correct payload and `X-GSM-Secret` header | -| Main server webhook call times out | Error logged; no retry in v1; service continues | -| `SMS_SENT|+639171234567` | Resolves the one in-flight request; the waiting sender persists the final result | -| `handle_inbound("ERR:101:timeout\n")` | Matching outbox row updated to `failed` | -| Serial line that does not parse | `ProtocolError` caught; error logged; service continues | +Run this only on a host with the configured Arduino and SIM: ---- +1. Set `DB_PATH`, `GSM_SECRET`, `SERIAL_PORT`, and `SMS_SEND_QUEUE_MAXSIZE` in the host environment file. +2. Start the GSM service and wait for `GSM_READY`. +3. Check liveness: -## Test File Locations + ```bash + curl http://127.0.0.1:8001/health + ``` -- `tests/test_config.py`: queue-capacity defaults and invalid settings. -- `tests/test_serial_worker.py`: capacity, lifecycle cutoff, exact-once completion, shutdown draining, and serial write timeout. -- `tests/test_api_queue.py`: 503 queue and shutdown contracts plus saturation diagnostics. +4. Send one SMS to a controlled test number: -``` -GSM-module/GSM-fastapi/ - tests/ - test_config.py - test_serial_worker.py - test_api_queue.py - -server/ - tests/ - test_gsm_otp.py - test_gsm_inbound_webhook.py -``` + ```bash + curl -X POST http://127.0.0.1:8001/sms/send \ + -H 'Content-Type: application/json' \ + -d '{"number":"+639171234567","body":"SAPOT GSM smoke test"}' + ``` -## Important: No Real Hardware in CI +5. Confirm the API response, `sms_log` status, Arduino event, and receipt on the test phone. -All CI runs must set: -``` -MOCK_SERIAL=true -GSM_SECRET=test-gsm-secret-value -DB_PATH=:memory: -``` +## Limitations -Any test that attempts to open `/dev/ttyACM0` or make an outbound HTTP call to a non-mocked host must fail the test suite with a clear error message. +- Automated tests do not prove USB permissions, modem readiness, SIM balance, signal quality, or carrier delivery. +- API tests mock message-log persistence; they do not validate the MariaDB schema. +- The suite does not test whether the main server preserves the direct gateway's HTTP status. It currently does not. +- Real-hardware testing must use a controlled phone number and must not run in shared CI. From dec0c6925c6083f47a69a4a5779bb45d2f060bfd Mon Sep 17 00:00:00 2001 From: Adamskiee Date: Tue, 11 Aug 2026 17:20:26 +0800 Subject: [PATCH 09/15] docs(gsm-deployment): fix callback troubleshooting link --- docs/getting-started/gsm-module-setup.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/getting-started/gsm-module-setup.md b/docs/getting-started/gsm-module-setup.md index 9d34312c..a6575cd7 100644 --- a/docs/getting-started/gsm-module-setup.md +++ b/docs/getting-started/gsm-module-setup.md @@ -85,4 +85,4 @@ quickest way to confirm the `.env` was picked up. - [docker-setup.md](docker-setup.md): the server must have a matching `GSM_SECRET` for inbound GSM callbacks. - [data-flow.md](../architecture/data-flow.md#sms-fallback) — the end-to-end SMS fallback flow diagram. -- [TROUBLESHOOTING.md](../TROUBLESHOOTING.md#gsm-module-and-server-cant-authenticate-each-other): when the two sides reject each other's webhooks. +- [TROUBLESHOOTING.md](../TROUBLESHOOTING.md#server-rejects-gsm-inbound-callbacks): when the server rejects the gateway's callback secret. From 1ddde674a85decf1c94547d44f7c9861b942b2fe Mon Sep 17 00:00:00 2001 From: Adamskiee Date: Tue, 11 Aug 2026 17:25:09 +0800 Subject: [PATCH 10/15] fix(deploy-gsm): restrict direct API to loopback --- docker-compose.yml | 2 +- docs/deployment/gsm-module.md | 2 +- docs/getting-started/docker-setup.md | 8 +++++--- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 0eaab2eb..b4142ae1 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -136,7 +136,7 @@ services: db: condition: service_healthy ports: - - "${GSM_FASTAPI_PORT:-8001}:8001" + - "127.0.0.1:${GSM_FASTAPI_PORT:-8001}:8001" volumes: db-data: diff --git a/docs/deployment/gsm-module.md b/docs/deployment/gsm-module.md index 0aee24a4..e4937618 100644 --- a/docs/deployment/gsm-module.md +++ b/docs/deployment/gsm-module.md @@ -34,7 +34,7 @@ bash run-api.sh ### Docker (dev/test alternative) -The root `docker-compose.yml` (see [docker-setup.md](../getting-started/docker-setup.md)) includes a `gsm-fastapi` service alongside the rest of the stack. The base file does not pass through `/dev/ttyACM0`, which lets development stacks start without GSM hardware. Add `docker-compose.gsm-hardware.yml` when the modem is attached. The Compose service already sets `HOST=0.0.0.0` so its published port is reachable from the host. +The root `docker-compose.yml` (see [docker-setup.md](../getting-started/docker-setup.md)) includes a `gsm-fastapi` service alongside the rest of the stack. The base file does not pass through `/dev/ttyACM0`, which lets development stacks start without GSM hardware. Add `docker-compose.gsm-hardware.yml` when the modem is attached. The service listens on all interfaces inside its container, but Compose publishes port 8001 only on host loopback because the direct API is unauthenticated. --- diff --git a/docs/getting-started/docker-setup.md b/docs/getting-started/docker-setup.md index b2652102..91a46272 100644 --- a/docs/getting-started/docker-setup.md +++ b/docs/getting-started/docker-setup.md @@ -86,8 +86,9 @@ that includes: (through `nginx`), **not** at the bare `/`, which 404s. - `tileserver`: offline map tiles. Not published to the host: `docker-compose.yml` only `expose`s port 8080 on the internal network, so reach it at `https://localhost/tiles/` through `nginx`. -- `gsm-fastapi`: the SMS gateway, `http://localhost:8001` (starts without the GSM modem; add - `docker-compose.gsm-hardware.yml` per the [Configure](#configure) section above for real SMS) +- `gsm-fastapi`: the SMS gateway, `http://localhost:8001` on host loopback only (starts without the + GSM modem; add `docker-compose.gsm-hardware.yml` per the [Configure](#configure) section above for + real SMS) `nginx` declares `depends_on` on `admin` and `tileserver` (both proxied by `nginx.docker.conf` as static upstreams, which nginx resolves at config-load time and refuses to start without). Naming @@ -180,7 +181,8 @@ both running at once: directory name, so a worktree gets its own containers, network, and `db-data` volume automatically — no shared state with the main checkout's stack. - **Host ports are not automatically isolated.** Two stacks (main checkout + a worktree, or two - worktrees) both bind `443`/`80`/`3000`/`8001` on the host by default, so bringing up a + worktrees) both bind `443`/`80`/`3000`/`8001` on the host by default. GSM port 8001 binds only to + loopback, while the other published services keep their configured interfaces. Bringing up a second stack while the first is still running fails with "port is already allocated". If you want them running concurrently, give the worktree its own `.env` (root-level, copied from `.env.example`) with different port values, e.g.: From 501a33b8ece98bb03dfafa3e58d26e9ac3442460 Mon Sep 17 00:00:00 2001 From: Adamskiee Date: Fri, 14 Aug 2026 14:13:20 +0800 Subject: [PATCH 11/15] fix(gsm-gateway): harden SMS delivery lifecycle --- GSM-module/CLAUDE.md | 6 +- GSM-module/GSM-fastapi/api.py | 35 ++++++- GSM-module/GSM-fastapi/config.py | 5 + GSM-module/GSM-fastapi/database.py | 11 +++ GSM-module/GSM-fastapi/serial_worker.py | 65 +++++++++---- GSM-module/GSM-fastapi/sms_handler.py | 55 +++++++---- GSM-module/GSM-fastapi/tests/conftest.py | 1 + .../GSM-fastapi/tests/test_api_queue.py | 47 +++++++-- GSM-module/GSM-fastapi/tests/test_config.py | 24 +++++ .../tests/test_database_reconciliation.py | 30 ++++++ .../GSM-fastapi/tests/test_incoming_sms.py | 52 ++++++++++ GSM-module/GSM-fastapi/tests/test_lifespan.py | 40 ++++++++ .../GSM-fastapi/tests/test_serial_worker.py | 95 +++++++++++++++++++ 13 files changed, 415 insertions(+), 51 deletions(-) create mode 100644 GSM-module/GSM-fastapi/tests/test_database_reconciliation.py create mode 100644 GSM-module/GSM-fastapi/tests/test_incoming_sms.py create mode 100644 GSM-module/GSM-fastapi/tests/test_lifespan.py diff --git a/GSM-module/CLAUDE.md b/GSM-module/CLAUDE.md index 28d6d050..87aab3e8 100644 --- a/GSM-module/CLAUDE.md +++ b/GSM-module/CLAUDE.md @@ -4,7 +4,7 @@ Instructions for Claude Code working in `GSM-module/` — SAPOT's SMS gateway, b ## Project Overview -Three layers: Arduino firmware talking AT commands to a SIM800L/SIM900 modem over serial; a Python (FastAPI) service on the same machine talking to the Arduino over USB serial; the main `server/` proxying outbound calls over trusted HTTP and authenticating inbound callbacks with a shared secret. There are **two parallel Python implementations** in this directory; they are not both live (see Architecture). +Three layers: Arduino firmware talking AT commands to a SIM800L/SIM900 modem over serial; a Python (FastAPI) service on the same machine talking to the Arduino over USB serial; the main `server/` proxying outbound calls and receiving inbound callbacks over HTTP authenticated with a shared secret. There are **two parallel Python implementations** in this directory; they are not both live (see Architecture). ## Architecture — which implementation is live @@ -18,7 +18,7 @@ Three layers: Arduino firmware talking AT commands to a SIM800L/SIM900 modem ove **Inbound:** Arduino emits `SMS_RECEIVED||` over serial → `serial_worker.py`'s `SerialWorker._reader_loop` parses it via `protocol.py` → queued → `api.py`'s async `_inbox_drain()` task offloads to a thread pool → `sms_handler.handle_incoming_sms()` (session/target flow, ban/verified checks against MariaDB) → `database.py`'s `notify_app()` POSTs to the main server's `/gsm/inbound` with an `X-GSM-Secret` header. -**Outbound:** the main server calls `POST /sms/send` on port 8001. `SerialWorker.send_sms()` atomically admits the request to a bounded FIFO queue or rejects saturation with HTTP 503. The sender writes `SEND_SMS||`, and the reader resolves the request from `SMS_SENT|` or `SMS_FAILED|`. The admin GSM page reads health and message history through the main server. Shutdown rejects queued and active work with `SERVICE_STOPPING`. +**Outbound:** the main server calls `POST /sms/send` on port 8001 with `X-GSM-Secret`. The gateway validates the secret before `SerialWorker.send_sms()` atomically admits the request to a bounded FIFO queue or rejects saturation with HTTP 503. The sender writes `SEND_SMS||`, and the reader resolves the request from `SMS_SENT|` or `SMS_FAILED|`. The admin GSM page reads health and message history through the main server. Shutdown rejects queued and active work with `SERVICE_STOPPING`. `SerialWorker` runs two dedicated threads (`_reader_loop`, `_sender_loop`) with proper request/response correlation over the async serial stream, and auto-reconnects every 10s on disconnect. @@ -33,7 +33,7 @@ Three layers: Arduino firmware talking AT commands to a SIM800L/SIM900 modem ove - **Wire protocol** — pipe-delimited lines: `SEND_SMS||` (PC → Arduino), `SMS_RECEIVED||`, `SMS_SENT|`, `SMS_FAILED||`, `GSM_READY`, `NETWORK_OK`/`NETWORK_LOST`, `SIM_MISSING` (Arduino → PC). Implemented identically in `GSM-fastapi/protocol.py`. Any change to this format must be mirrored in the `.ino` firmware's parser/emitter — they are independent implementations of the same contract, not shared code. - **Session/target flow** — inbound SMS starts a session (`NEW`), the sender texts `[target] +63...` to select a recipient (`AWAITING_TARGET` → `ACTIVE`), then messages relay through. `GSM-fastapi` persists this to MariaDB (`SmsSession` table) and checks `banned`/`phone_is_verified` on both sender and target; `GSM-API`'s equivalent is in-memory only and skips those checks. -- **Shared-secret webhook auth** (`X-GSM-Secret` header) — how this service calls back into the main server (`/gsm/inbound`); distinct from the JWT auth used elsewhere in SAPOT (see `../server/CLAUDE.md`). +- **Shared-secret service auth** (`X-GSM-Secret` header) — authenticates main-server calls to `/sms/send` and GSM callbacks to `/gsm/inbound`; distinct from the JWT auth used elsewhere in SAPOT (see `../server/CLAUDE.md`). ## Development Conventions diff --git a/GSM-module/GSM-fastapi/api.py b/GSM-module/GSM-fastapi/api.py index 37e06d6a..6fd2c59d 100644 --- a/GSM-module/GSM-fastapi/api.py +++ b/GSM-module/GSM-fastapi/api.py @@ -27,12 +27,13 @@ """ import asyncio +import hmac import logging import re from contextlib import asynccontextmanager -from typing import Counter, Optional +from typing import Annotated, Counter, Optional -from fastapi import FastAPI, HTTPException, Query, BackgroundTasks +from fastapi import BackgroundTasks, Depends, FastAPI, Header, HTTPException, Query from fastapi.responses import JSONResponse from pydantic import BaseModel, field_validator @@ -61,6 +62,12 @@ async def lifespan(app: FastAPI): # Database database.init(settings.db_path) logger.info("Database ready") + orphaned_count = database.fail_orphaned_pending_messages() + if orphaned_count: + logger.warning( + "Marked %d orphaned SMS messages as failed after service restart", + orphaned_count, + ) # Serial worker _worker = SerialWorker( @@ -127,7 +134,12 @@ def _process_incoming(event): status="received", ) - reply, forward_number, forward_body = handle_incoming_sms(number, body) + reply, forward_number, forward_body, rejection_reason = handle_incoming_sms( + number, body + ) + + if rejection_reason: + database.update_message_status(msg_id, "rejected", rejection_reason) # Forward to target via SMS if forward_number and forward_body and _worker: @@ -217,6 +229,17 @@ def validate_phone(cls, v): return v +def require_gsm_secret( + x_gsm_secret: Annotated[ + Optional[str], Header(alias="X-GSM-Secret") + ] = None, +): + if x_gsm_secret is None or not hmac.compare_digest( + x_gsm_secret, settings.gsm_secret + ): + raise HTTPException(status_code=401, detail="Invalid GSM secret") + + # ── Health endpoints ────────────────────────────────────────────────────────── @app.get("/health", tags=["health"]) @@ -320,7 +343,11 @@ def status(): # ── SMS endpoints ───────────────────────────────────────────────────────────── -@app.post("/sms/send", tags=["sms"]) +@app.post( + "/sms/send", + tags=["sms"], + dependencies=[Depends(require_gsm_secret)], +) def send_sms(req: SendSMSRequest): """ Send an SMS directly. Blocks until the modem confirms delivery. diff --git a/GSM-module/GSM-fastapi/config.py b/GSM-module/GSM-fastapi/config.py index 34b64921..1fd0dcd4 100644 --- a/GSM-module/GSM-fastapi/config.py +++ b/GSM-module/GSM-fastapi/config.py @@ -44,6 +44,11 @@ class Settings: if not db_path: raise RuntimeError("Environment variable 'DB_PATH' is not set.") + gsm_secret: str = os.environ.get("GSM_SECRET", "") + + if not gsm_secret: + raise RuntimeError("Environment variable 'GSM_SECRET' is not set.") + # FastAPI host and port host: str = os.environ.get("HOST", "127.0.0.1") port: int = int(os.environ.get("PORT", "8000")) diff --git a/GSM-module/GSM-fastapi/database.py b/GSM-module/GSM-fastapi/database.py index 469ab883..6fce6193 100644 --- a/GSM-module/GSM-fastapi/database.py +++ b/GSM-module/GSM-fastapi/database.py @@ -520,6 +520,17 @@ def update_message_status(msg_id: str, status: str, s.commit() +def fail_orphaned_pending_messages() -> int: + with new_get_session() as s: + result = s.execute( + update(SmsLog) + .where(SmsLog.status == "pending") + .values(status="failed", failure_reason="SERVICE_CRASHED") + ) + s.commit() + return result.rowcount + + def get_messages(limit: int = 50, offset: int = 0, direction: Optional[str] = None, phone: Optional[str] = None) -> dict: with new_get_session() as s: diff --git a/GSM-module/GSM-fastapi/serial_worker.py b/GSM-module/GSM-fastapi/serial_worker.py index 9b1fcbf8..dd2fa080 100644 --- a/GSM-module/GSM-fastapi/serial_worker.py +++ b/GSM-module/GSM-fastapi/serial_worker.py @@ -52,8 +52,14 @@ class _SendRequest: body: str timeout: float done: threading.Event = field(default_factory=threading.Event) + completion_lock: threading.Lock = field(default_factory=threading.Lock) success: bool = False reason: Optional[str] = None + deadline: float = field(init=False) + write_started: bool = False + + def __post_init__(self): + self.deadline = time.monotonic() + max(0, self.timeout) class SerialWorker: @@ -151,6 +157,7 @@ def send_sms(self, number: str, body: str, raise RuntimeError("GSM modem not ready") try: self._send_queue.put_nowait(req) + req.deadline = time.monotonic() + max(0, req.timeout) except queue.Full as error: logger.warning( "Outbound SMS queue full (depth=%d capacity=%d)", @@ -159,10 +166,12 @@ def send_sms(self, number: str, body: str, logger.info("SMS enqueued to %s (queue depth %d)", number, self._send_queue.qsize()) - # Block until the sender + reader threads resolve this request - delivered = req.done.wait(timeout=timeout + 5) # +5 s buffer - if not delivered: - return {"ok": False, "reason": "CLIENT_TIMEOUT"} + while not req.done.is_set(): + remaining = max(0, req.deadline - time.monotonic()) + if req.done.wait(timeout=remaining): + break + if not self._timeout_request(req): + req.done.wait(timeout=0.01) return {"ok": req.success, "reason": req.reason} @property @@ -199,13 +208,17 @@ def _sender_loop(self): with self._active_lock: self._active_request = req - # Wait until modem is ready (e.g. after reconnect) - deadline = time.time() + req.timeout - while not self.gsm_ready and time.time() < deadline: - time.sleep(0.5) + while (not self.gsm_ready and not req.done.is_set() + and time.monotonic() < req.deadline): + remaining = req.deadline - time.monotonic() + req.done.wait(timeout=min(0.5, max(0, remaining))) + + if req.done.is_set(): + self._clear_active_request(req) + continue if not self.gsm_ready: - self._complete_active_request(req, False, "MODEM_NOT_READY") + self._complete_active_request(req, False, "TIMEOUT") logger.warning("SMS to %s dropped: modem not ready", req.number) continue @@ -214,9 +227,8 @@ def _sender_loop(self): self._clear_active_request(req) continue - # Block here until the reader resolves this request - # (or until the per-SMS timeout expires) - resolved = req.done.wait(timeout=req.timeout) + remaining = max(0, req.deadline - time.monotonic()) + resolved = req.done.wait(timeout=remaining) if not resolved: if self._complete_in_flight(req, False, "TIMEOUT"): logger.error("SMS to %s timed out", req.number) @@ -363,8 +375,12 @@ def _write_active_request(self, req: _SendRequest, cmd: str) -> bool: with self._active_lock: if self._active_request is not req or req.done.is_set(): return False + if time.monotonic() >= req.deadline: + self._complete_request(req, False, "TIMEOUT") + return False with self._in_flight_lock: self._in_flight = req + req.write_started = True try: with self._ser_lock: if not self._ser or not self._ser.is_open: @@ -375,10 +391,23 @@ def _write_active_request(self, req: _SendRequest, cmd: str) -> bool: self._in_flight = None self._complete_request(req, False, f"WRITE_ERROR: {error}") return False + req.deadline = time.monotonic() + max(0, req.timeout) logger.info("SMS sent to serial: to=%s body=%r", req.number, req.body) return True + def _timeout_request(self, req: _SendRequest) -> bool: + with self._active_lock: + if req.done.is_set(): + return True + if req.write_started: + return False + with self._in_flight_lock: + if self._in_flight is req: + self._in_flight = None + self._complete_request(req, False, "CLIENT_TIMEOUT") + return True + def _fail_active_request(self, reason: str): with self._active_lock: req = self._active_request @@ -418,10 +447,14 @@ def _complete_in_flight(self, req: _SendRequest, success: bool, @staticmethod def _complete_request(req: _SendRequest, success: bool, - reason: Optional[str]): - req.success = success - req.reason = reason - req.done.set() + reason: Optional[str]) -> bool: + with req.completion_lock: + if req.done.is_set(): + return False + req.success = success + req.reason = reason + req.done.set() + return True def _drain_queued_requests(self): while True: diff --git a/GSM-module/GSM-fastapi/sms_handler.py b/GSM-module/GSM-fastapi/sms_handler.py index b439ddce..a16473e1 100644 --- a/GSM-module/GSM-fastapi/sms_handler.py +++ b/GSM-module/GSM-fastapi/sms_handler.py @@ -3,7 +3,7 @@ ────────────── Business logic for incoming SMS messages. -Returns (reply, forward_number, forward_body) — all strings or None. +Returns (reply, forward_number, forward_body, rejection_reason). The caller (api.py _process_incoming) sends them via the serial worker. Message length rule: every constant must stay under 160 chars @@ -68,8 +68,8 @@ def _forward_body(sender_phone: str, body: str) -> str: # ── Types ───────────────────────────────────────────────────────────────────── -ForwardTuple = Tuple[Optional[str], Optional[str], Optional[str]] -# (reply_to_sender, forward_to_number, forward_body) +ForwardTuple = Tuple[Optional[str], Optional[str], Optional[str], Optional[str]] +# (reply_to_sender, forward_to_number, forward_body, rejection_reason) # ── Main entry point ────────────────────────────────────────────────────────── @@ -81,18 +81,28 @@ def handle_incoming_sms(number: str, body: str) -> ForwardTuple: if not sender_user: logger.warning("Account does not exist: %s", number) - return MSG_NO_ACCOUNT, None, None + return MSG_NO_ACCOUNT, None, None, "NO_ACCOUNT" if sender_user.get("banned"): logger.warning("Banned: %s", number) - return "This number has been banned by the system", None, None + return ( + "This number has been banned by the system", + None, + None, + "BANNED_SENDER", + ) if not sender_user.get("phone_is_verified"): logger.warning("Unverified number: %s", number) - return "Please verify your account first.", None, None + return ( + "Please verify your account first.", + None, + None, + "UNVERIFIED_SENDER", + ) sender = database.lookup_number(number) if sender is None: - return MSG_NO_ACCOUNT, None, None + return MSG_NO_ACCOUNT, None, None, "NO_ACCOUNT" session = database.get_session(number) stage = session["stage"] @@ -103,16 +113,16 @@ def handle_incoming_sms(number: str, body: str) -> ForwardTuple: if stage == "NEW": database.update_session(number, stage="AWAITING_TARGET") - return MSG_WELCOME, None, None + return MSG_WELCOME, None, None, None if stage == "AWAITING_TARGET": - return MSG_NEED_TARGET, None, None + return MSG_NEED_TARGET, None, None, None if stage == "ACTIVE": return _do_forward(number, body, session) database.reset_session(number) - return MSG_WELCOME, None, None + return MSG_WELCOME, None, None, None # ── Sub-handlers ────────────────────────────────────────────────────────────── @@ -121,20 +131,20 @@ def _cmd_set_target(number: str, body: str) -> ForwardTuple: # body is like "[target] +639281234567" or "[target]" (no arg) parts = body.split(None, 1) if len(parts) < 2 or not parts[1].strip(): - return MSG_NO_ARG, None, None + return MSG_NO_ARG, None, None, None target_phone = parts[1].strip() if not target_phone.startswith("+") or not target_phone[1:].isdigit(): - return MSG_INVALID_FMT, None, None + return MSG_INVALID_FMT, None, None, None # Sender cannot target themselves if target_phone == number: - return "You cannot set yourself as the target.", None, None + return "You cannot set yourself as the target.", None, None, None target = database.lookup_number(target_phone) if target is None: - return MSG_TARGET_NOT_FOUND, None, None + return MSG_TARGET_NOT_FOUND, None, None, None database.update_session( number, @@ -143,7 +153,7 @@ def _cmd_set_target(number: str, body: str) -> ForwardTuple: target_username=target["username"], ) - return _msg_target_set(target["username"], target_phone), None, None + return _msg_target_set(target["username"], target_phone), None, None, None def _do_forward(sender_phone: str, body: str, session: dict) -> ForwardTuple: @@ -154,18 +164,23 @@ def _do_forward(sender_phone: str, body: str, session: dict) -> ForwardTuple: if not target_user: logger.warning("Target does not exist: %s", target_phone) - return f"Target {target_phone} does not exist.", sender_phone, None + return f"Target {target_phone} does not exist.", sender_phone, None, None if target_user.get("banned"): logger.warning("Banned: %s", target_phone) - return f"This number ({target_phone}) has been banned by the system.", None, None + return ( + f"This number ({target_phone}) has been banned by the system.", + None, + None, + None, + ) if not target_user.get("phone_is_verified"): logger.warning("Unverified number: %s", target_phone) - return f"Target {target_phone} is not verified.", None, None + return f"Target {target_phone} is not verified.", None, None, None if not target_phone: database.reset_session(sender_phone) - return MSG_WELCOME, None, None + return MSG_WELCOME, None, None, None ok = database.notify_app(sender_phone, target_phone, body) logger.info("notify_app result: %s (sender=%s target=%s)", ok, sender_phone, target_phone) @@ -173,4 +188,4 @@ def _do_forward(sender_phone: str, body: str, session: dict) -> ForwardTuple: # Build clean forward body for the target's SMS fwd = _forward_body(sender_phone, body) - return _msg_forwarded(target_username), target_phone, fwd + return _msg_forwarded(target_username), target_phone, fwd, None diff --git a/GSM-module/GSM-fastapi/tests/conftest.py b/GSM-module/GSM-fastapi/tests/conftest.py index 7ad3262d..e2e8a64b 100644 --- a/GSM-module/GSM-fastapi/tests/conftest.py +++ b/GSM-module/GSM-fastapi/tests/conftest.py @@ -3,4 +3,5 @@ from pathlib import Path os.environ.setdefault("DB_PATH", "sqlite:///./test-gsm.db") +os.environ.setdefault("GSM_SECRET", "test-gsm-secret") sys.path.insert(0, str(Path(__file__).resolve().parents[1])) diff --git a/GSM-module/GSM-fastapi/tests/test_api_queue.py b/GSM-module/GSM-fastapi/tests/test_api_queue.py index 94375bb6..72804f74 100644 --- a/GSM-module/GSM-fastapi/tests/test_api_queue.py +++ b/GSM-module/GSM-fastapi/tests/test_api_queue.py @@ -6,12 +6,15 @@ import pytest import api +from config import settings from serial_worker import ( MAX_SEND_QUEUE_SIZE, OutboundQueueFullError, WorkerStoppingError, ) +AUTH_HEADERS = {"X-GSM-Secret": settings.gsm_secret} + @pytest.fixture(autouse=True) def reset_worker(): @@ -41,9 +44,11 @@ def test_queue_full_returns_nested_503_and_updates_log(monkeypatch): monkeypatch.setattr(api.database, "update_message_status", lambda *args: updates.append(args)) api._worker = RejectingWorker(OutboundQueueFullError()) - response = TestClient(api.app).post("/sms/send", json={ - "number": "+639171234567", "body": "message" - }) + response = TestClient(api.app).post( + "/sms/send", + json={"number": "+639171234567", "body": "message"}, + headers=AUTH_HEADERS, + ) assert response.status_code == 503 assert response.json()["detail"] == { @@ -60,9 +65,11 @@ def test_stopping_returns_nested_503_and_updates_log(monkeypatch): monkeypatch.setattr(api.database, "update_message_status", lambda *args: updates.append(args)) api._worker = RejectingWorker(WorkerStoppingError()) - response = TestClient(api.app).post("/sms/send", json={ - "number": "+639171234567", "body": "message" - }) + response = TestClient(api.app).post( + "/sms/send", + json={"number": "+639171234567", "body": "message"}, + headers=AUTH_HEADERS, + ) assert response.status_code == 503 assert response.json()["detail"]["reason"] == "SERVICE_STOPPING" @@ -116,7 +123,9 @@ async def exercise_saturation(): transport = httpx.ASGITransport(app=api.app) async with httpx.AsyncClient(transport=transport, base_url="http://test") as client: payload = {"number": "+639171234567", "body": "message"} - admitted = [asyncio.create_task(client.post("/sms/send", json=payload)) + admitted = [asyncio.create_task(client.post( + "/sms/send", json=payload, headers=AUTH_HEADERS + )) for _ in range(worker.capacity)] for _ in range(100): @@ -126,7 +135,8 @@ async def exercise_saturation(): await asyncio.sleep(0.01) rejected = await asyncio.wait_for( - client.post("/sms/send", json=payload), timeout=1 + client.post("/sms/send", json=payload, headers=AUTH_HEADERS), + timeout=1, ) health = await asyncio.wait_for(client.get("/health"), timeout=1) worker.release.set() @@ -140,3 +150,24 @@ async def exercise_saturation(): assert rejected.json()["detail"]["reason"] == "QUEUE_FULL" assert health.status_code == 200 assert all(response.status_code == 200 for response in completed) + + +@pytest.mark.parametrize("headers", [{}, {"X-GSM-Secret": "wrong-secret"}]) +def test_send_rejects_missing_or_invalid_secret_before_side_effects( + monkeypatch, headers +): + calls = [] + monkeypatch.setattr( + api.database, "log_message", lambda **_kwargs: calls.append("logged") + ) + api._worker = RejectingWorker(AssertionError("worker must not be called")) + + response = TestClient(api.app).post( + "/sms/send", + json={"number": "+639171234567", "body": "message"}, + headers=headers, + ) + + assert response.status_code == 401 + assert response.json() == {"detail": "Invalid GSM secret"} + assert calls == [] diff --git a/GSM-module/GSM-fastapi/tests/test_config.py b/GSM-module/GSM-fastapi/tests/test_config.py index c0dd62e4..036d4639 100644 --- a/GSM-module/GSM-fastapi/tests/test_config.py +++ b/GSM-module/GSM-fastapi/tests/test_config.py @@ -1,3 +1,8 @@ +import os +from pathlib import Path +import subprocess +import sys + import pytest from config import bounded_integer_env @@ -22,3 +27,22 @@ def test_bounded_integer_env_accepts_value_in_range(monkeypatch, value): monkeypatch.setenv("TEST_QUEUE_SIZE", value) assert bounded_integer_env("TEST_QUEUE_SIZE", 10, 20) == int(value) + + +def test_config_rejects_missing_gsm_secret(tmp_path): + env = os.environ.copy() + env["DB_PATH"] = "sqlite:///test.db" + env.pop("GSM_SECRET", None) + env["PYTHONPATH"] = str(Path(__file__).resolve().parents[1]) + + result = subprocess.run( + [sys.executable, "-c", "import config"], + cwd=tmp_path, + env=env, + capture_output=True, + text=True, + check=False, + ) + + assert result.returncode != 0 + assert "Environment variable 'GSM_SECRET' is not set." in result.stderr diff --git a/GSM-module/GSM-fastapi/tests/test_database_reconciliation.py b/GSM-module/GSM-fastapi/tests/test_database_reconciliation.py new file mode 100644 index 00000000..b884e81f --- /dev/null +++ b/GSM-module/GSM-fastapi/tests/test_database_reconciliation.py @@ -0,0 +1,30 @@ +import database + + +def test_fail_orphaned_pending_messages_marks_only_pending_rows(tmp_path): + database.init(f"sqlite:///{tmp_path / 'gsm.db'}") + pending_id = database.log_message( + direction="OUT", + from_number="API", + to_number="+639171234567", + body="pending message", + ) + received_id = database.log_message( + direction="IN", + from_number="+639171234568", + to_number="SERVER", + body="received message", + status="received", + ) + + assert database.fail_orphaned_pending_messages() == 1 + + messages = { + message["id"]: message + for message in database.get_messages(limit=10)["messages"] + } + assert messages[pending_id]["status"] == "failed" + assert messages[pending_id]["failure_reason"] == "SERVICE_CRASHED" + assert messages[received_id]["status"] == "received" + assert messages[received_id]["failure_reason"] is None + assert database.fail_orphaned_pending_messages() == 0 diff --git a/GSM-module/GSM-fastapi/tests/test_incoming_sms.py b/GSM-module/GSM-fastapi/tests/test_incoming_sms.py new file mode 100644 index 00000000..c9348813 --- /dev/null +++ b/GSM-module/GSM-fastapi/tests/test_incoming_sms.py @@ -0,0 +1,52 @@ +from types import SimpleNamespace + +import pytest + +import api +import sms_handler + + +@pytest.mark.parametrize( + ("sender", "expected_reason"), + [ + (None, "NO_ACCOUNT"), + ({"banned": True, "phone_is_verified": True}, "BANNED_SENDER"), + ({"banned": False, "phone_is_verified": False}, "UNVERIFIED_SENDER"), + ], +) +def test_handle_incoming_sms_reports_sender_rejection_reason( + monkeypatch, sender, expected_reason +): + monkeypatch.setattr( + sms_handler.database, "get_user_by_phone", lambda _number: sender + ) + + reply, forward_number, forward_body, rejection_reason = ( + sms_handler.handle_incoming_sms("+639171234567", "help") + ) + + assert reply + assert forward_number is None + assert forward_body is None + assert rejection_reason == expected_reason + + +def test_process_incoming_persists_rejection_reason(monkeypatch): + updates = [] + monkeypatch.setattr(api.database, "log_message", lambda **_kwargs: "message-id") + monkeypatch.setattr( + api.database, + "update_message_status", + lambda *args: updates.append(args), + ) + monkeypatch.setattr( + api, + "handle_incoming_sms", + lambda _number, _body: (None, None, None, "BANNED_SENDER"), + ) + + api._process_incoming( + SimpleNamespace(number="+639171234567", body="blocked message") + ) + + assert updates == [("message-id", "rejected", "BANNED_SENDER")] diff --git a/GSM-module/GSM-fastapi/tests/test_lifespan.py b/GSM-module/GSM-fastapi/tests/test_lifespan.py new file mode 100644 index 00000000..60b0e72d --- /dev/null +++ b/GSM-module/GSM-fastapi/tests/test_lifespan.py @@ -0,0 +1,40 @@ +import asyncio +import queue + +import api + + +def test_lifespan_reconciles_pending_messages_before_starting_worker(monkeypatch): + events = [] + + class FakeWorker: + def __init__(self, *_args): + events.append("worker_created") + self.incoming_queue = queue.Queue() + + def start(self): + events.append("worker_started") + + def stop(self): + events.append("worker_stopped") + + monkeypatch.setattr(api.database, "init", lambda _path: events.append("db_ready")) + monkeypatch.setattr( + api.database, + "fail_orphaned_pending_messages", + lambda: events.append("pending_reconciled") or 2, + ) + monkeypatch.setattr(api, "SerialWorker", FakeWorker) + + async def run_lifespan(): + async with api.lifespan(api.app): + assert events == [ + "db_ready", + "pending_reconciled", + "worker_created", + "worker_started", + ] + + asyncio.run(run_lifespan()) + + assert events[-1] == "worker_stopped" diff --git a/GSM-module/GSM-fastapi/tests/test_serial_worker.py b/GSM-module/GSM-fastapi/tests/test_serial_worker.py index 82df4130..db82f464 100644 --- a/GSM-module/GSM-fastapi/tests/test_serial_worker.py +++ b/GSM-module/GSM-fastapi/tests/test_serial_worker.py @@ -136,6 +136,101 @@ def write(self, payload): assert writes == [] +def test_queued_request_timed_out_by_caller_is_never_written(): + writes = [] + + class FakeSerial: + is_open = True + + def write(self, payload): + writes.append(payload) + + worker = ready_worker(capacity=2) + worker._ser = FakeSerial() + first = _SendRequest("+639171234567", "first", 10) + worker._send_queue.put_nowait(first) + sender = threading.Thread(target=worker._sender_loop) + sender.start() + + for _ in range(100): + if writes: + break + time.sleep(0.01) + + result = worker.send_sms("+639171234568", "second", timeout=0.01) + worker._complete_in_flight(first, True, None) + + for _ in range(100): + if worker.outbound_queue_depth == 0: + break + time.sleep(0.01) + worker._stop.set() + sender.join(timeout=2) + + assert result == {"ok": False, "reason": "CLIENT_TIMEOUT"} + assert writes == [b"SEND_SMS|+639171234567|first\n"] + assert sender.is_alive() is False + + +def test_stop_does_not_overwrite_completed_queue_timeout(): + worker = ready_worker() + + result = worker.send_sms("+639171234568", "message", timeout=0.01) + request = worker._send_queue.get_nowait() + worker._send_queue.put_nowait(request) + + class JoinedThread: + def join(self, timeout): + assert timeout == 5 + + worker._reader_thread = JoinedThread() + worker._sender_thread = JoinedThread() + worker.stop() + + assert result == {"ok": False, "reason": "CLIENT_TIMEOUT"} + assert request.reason == "CLIENT_TIMEOUT" + + +def test_write_crossing_admission_deadline_waits_for_confirmation(): + writes = [] + write_started = threading.Event() + release_write = threading.Event() + + class BlockingSerial: + is_open = True + + def write(self, payload): + write_started.set() + release_write.wait(timeout=1) + writes.append(payload) + + worker = ready_worker() + worker._ser = BlockingSerial() + sender = threading.Thread(target=worker._sender_loop) + sender.start() + result = {} + + def send(): + result.update( + worker.send_sms("+639171234568", "message", timeout=0.05) + ) + + caller = threading.Thread(target=send) + caller.start() + assert write_started.wait(timeout=1) + time.sleep(0.06) + release_write.set() + worker._resolve_in_flight("+639171234568", True, None) + caller.join(timeout=1) + worker._stop.set() + sender.join(timeout=2) + + assert result == {"ok": True, "reason": None} + assert writes == [b"SEND_SMS|+639171234568|message\n"] + assert caller.is_alive() is False + assert sender.is_alive() is False + + def test_stale_confirmation_cannot_complete_request_before_write(): writes = [] From 33740701a6181a0046612b7ba5c737feb2b3955e Mon Sep 17 00:00:00 2001 From: Adamskiee Date: Fri, 14 Aug 2026 14:13:29 +0800 Subject: [PATCH 12/15] fix(server-gsm): preserve gateway failure responses --- deploy/config/nginx.prod.conf | 2 +- docker/nginx.docker.conf | 2 +- docs/api/openapi/gsm-sms.yaml | 134 +++++++++++++++ server/app/api/gsm.py | 105 ++++++++++-- server/app/tests/test_gsm_health.py | 28 ++++ server/app/tests/test_gsm_proxy.py | 247 ++++++++++++++++++++++++++++ server/nginx.conf | 2 +- 7 files changed, 500 insertions(+), 20 deletions(-) create mode 100644 server/app/tests/test_gsm_proxy.py diff --git a/deploy/config/nginx.prod.conf b/deploy/config/nginx.prod.conf index 80a80688..68adf794 100644 --- a/deploy/config/nginx.prod.conf +++ b/deploy/config/nginx.prod.conf @@ -11,6 +11,6 @@ server { location ~ ^/(data|fonts|sprites)/ { proxy_pass http://tileserver:8080; proxy_set_header Host $host; proxy_set_header X-Forwarded-Proto $scheme; proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; } location /admin { proxy_pass http://admin:3000; proxy_set_header Host $host; proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header X-Forwarded-Proto $scheme; } location /ws/ { proxy_pass http://api:8000; proxy_http_version 1.1; proxy_set_header Upgrade $http_upgrade; proxy_set_header Connection "upgrade"; proxy_set_header Host $host; proxy_set_header X-Forwarded-For $remote_addr; proxy_read_timeout 86400s; proxy_send_timeout 86400s; } - location / { proxy_set_header Authorization $http_authorization; proxy_pass_header Authorization; proxy_pass http://api:8000; proxy_set_header Host $host; proxy_read_timeout 135s; proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header X-Forwarded-Proto $scheme; proxy_redirect http:// https://; gzip on; gzip_types application/json text/plain; gzip_min_length 256; } + location / { proxy_set_header Authorization $http_authorization; proxy_pass_header Authorization; proxy_pass http://api:8000; proxy_set_header Host $host; proxy_read_timeout 155s; proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header X-Forwarded-Proto $scheme; proxy_redirect http:// https://; gzip on; gzip_types application/json text/plain; gzip_min_length 256; } } server { listen 80; listen [::]:80; return 301 https://$host$request_uri; } diff --git a/docker/nginx.docker.conf b/docker/nginx.docker.conf index 73fd9111..20cdb345 100644 --- a/docker/nginx.docker.conf +++ b/docker/nginx.docker.conf @@ -102,7 +102,7 @@ server { proxy_pass http://api:8000; proxy_set_header Host $host; - proxy_read_timeout 135s; + proxy_read_timeout 155s; proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header X-Forwarded-Proto $scheme; # ← tells backend it's HTTPS diff --git a/docs/api/openapi/gsm-sms.yaml b/docs/api/openapi/gsm-sms.yaml index 4e7dee7e..9656fe9b 100644 --- a/docs/api/openapi/gsm-sms.yaml +++ b/docs/api/openapi/gsm-sms.yaml @@ -31,6 +31,21 @@ paths: schema: {} '404': description: Not Found + '502': + description: The modem rejected the SMS or delivery confirmation timed out. + content: + application/json: + schema: + $ref: '#/components/schemas/GsmFailureResponse' + '503': + description: The GSM gateway is unavailable, stopping, or at queue capacity. + content: + application/json: + schema: + anyOf: + - $ref: '#/components/schemas/GsmFailureResponse' + - $ref: '#/components/schemas/GsmHealthUnavailableResponse' + title: Response 503 Contact Unknown User Gsm Contact Unknown User Post '422': description: Validation Error content: @@ -51,6 +66,15 @@ paths: schema: {} '404': description: Not Found + '503': + description: The GSM gateway is unavailable or reports degraded health. + content: + application/json: + schema: + anyOf: + - $ref: '#/components/schemas/GsmHealthResponse' + - $ref: '#/components/schemas/GsmHealthUnavailableResponse' + title: Response 503 Gsm Health Gsm Health Get security: - OAuth2PasswordBearer: [] /gsm/health/detailed: @@ -68,6 +92,15 @@ paths: schema: {} '404': description: Not Found + '503': + description: The GSM gateway is unavailable or reports degraded health. + content: + application/json: + schema: + anyOf: + - $ref: '#/components/schemas/GsmHealthResponse' + - $ref: '#/components/schemas/GsmHealthUnavailableResponse' + title: Response 503 Gsm Health Detailed Gsm Health Detailed Get security: - OAuth2PasswordBearer: [] /gsm/inbound: @@ -405,6 +438,21 @@ paths: schema: {} '404': description: Not Found + '502': + description: The modem rejected the SMS or delivery confirmation timed out. + content: + application/json: + schema: + $ref: '#/components/schemas/GsmFailureResponse' + '503': + description: The GSM gateway is unavailable, stopping, or at queue capacity. + content: + application/json: + schema: + anyOf: + - $ref: '#/components/schemas/GsmFailureResponse' + - $ref: '#/components/schemas/GsmHealthUnavailableResponse' + title: Response 503 Request Phone Verification Gsm Request Post '422': description: Validation Error content: @@ -428,6 +476,21 @@ paths: schema: {} '404': description: Not Found + '502': + description: The modem rejected the SMS or delivery confirmation timed out. + content: + application/json: + schema: + $ref: '#/components/schemas/GsmFailureResponse' + '503': + description: The GSM gateway is unavailable, stopping, or at queue capacity. + content: + application/json: + schema: + anyOf: + - $ref: '#/components/schemas/GsmFailureResponse' + - $ref: '#/components/schemas/GsmHealthUnavailableResponse' + title: Response 503 Resend Phone Code Gsm Resend Post security: - OAuth2PasswordBearer: [] /gsm/sms/messages: @@ -514,6 +577,21 @@ paths: schema: {} '404': description: Not Found + '502': + description: The modem rejected the SMS or delivery confirmation timed out. + content: + application/json: + schema: + $ref: '#/components/schemas/GsmFailureResponse' + '503': + description: The GSM gateway is unavailable, stopping, or at queue capacity. + content: + application/json: + schema: + anyOf: + - $ref: '#/components/schemas/GsmFailureResponse' + - $ref: '#/components/schemas/GsmHealthUnavailableResponse' + title: Response 503 Send Sms Gsm Sms Send Post '422': description: Validation Error content: @@ -579,6 +657,62 @@ paths: - OAuth2PasswordBearer: [] components: schemas: + GsmFailureDetail: + properties: + message: + type: string + title: Message + reason: + type: string + title: Reason + msg_id: + anyOf: + - type: string + - type: 'null' + title: Msg Id + type: object + required: + - message + - reason + title: GsmFailureDetail + GsmFailureResponse: + properties: + detail: + $ref: '#/components/schemas/GsmFailureDetail' + type: object + required: + - detail + title: GsmFailureResponse + GsmHealthResponse: + properties: + status: + type: string + title: Status + gsm_ready: + type: boolean + title: Gsm Ready + connected: + type: boolean + title: Connected + detail: + type: string + title: Detail + type: object + required: + - status + - gsm_ready + - connected + - detail + title: GsmHealthResponse + GsmHealthUnavailableResponse: + properties: + detail: + type: string + title: Detail + type: object + required: + - detail + title: GsmHealthUnavailableResponse HTTPValidationError: properties: detail: diff --git a/server/app/api/gsm.py b/server/app/api/gsm.py index 902bb71f..e630c72a 100644 --- a/server/app/api/gsm.py +++ b/server/app/api/gsm.py @@ -4,6 +4,7 @@ from uuid import UUID, uuid4, uuid5 from fastapi import Depends, HTTPException, Query, Request from fastapi.routing import APIRouter +from fastapi.responses import JSONResponse import time from pydantic import BaseModel @@ -22,8 +23,15 @@ import httpx import json +# The gateway allows 60 seconds before write, a 5-second serial write, and a +# fresh 60-second confirmation window. The proxy adds 10 seconds for HTTP overhead. +GSM_GATEWAY_WORST_CASE_SECONDS = 60.0 + 5.0 + 60.0 +GSM_PROXY_READ_TIMEOUT_SECONDS = GSM_GATEWAY_WORST_CASE_SECONDS + 10.0 +GSM_GATEWAY_MAX_ADMITTED_REQUESTS = 21 +GSM_PROXY_MAX_CONNECTIONS = GSM_GATEWAY_MAX_ADMITTED_REQUESTS + 1 +GSM_PROXY_POOL_TIMEOUT_SECONDS = 1.0 + # Module-level client reuses TCP connections to localhost:8001 across requests. -# The 120s timeout matches the SMS send worst-case; health checks are much faster. _gsm_http_client: httpx.AsyncClient | None = None logger = logging.getLogger("app") @@ -37,8 +45,16 @@ def _get_gsm_client() -> httpx.AsyncClient: if _gsm_http_client is None or _gsm_http_client.is_closed: _gsm_http_client = httpx.AsyncClient( base_url="http://localhost:8001", - timeout=120.0, - limits=httpx.Limits(max_connections=4, max_keepalive_connections=2), + timeout=httpx.Timeout( + connect=5.0, + read=GSM_PROXY_READ_TIMEOUT_SECONDS, + write=5.0, + pool=GSM_PROXY_POOL_TIMEOUT_SECONDS, + ), + limits=httpx.Limits( + max_connections=GSM_PROXY_MAX_CONNECTIONS, + max_keepalive_connections=4, + ), ) return _gsm_http_client @@ -52,7 +68,7 @@ def _gsm_log_extra(current_user: User | None, path: str) -> dict: } -async def _get_gsm_health(path: str, current_user: User) -> dict: +async def _get_gsm_health(path: str, current_user: User) -> dict | JSONResponse: try: response = await _get_gsm_client().get(path) except httpx.RequestError as exc: @@ -63,7 +79,10 @@ async def _get_gsm_health(path: str, current_user: User) -> dict: ) raise HTTPException(status_code=503, detail="GSM gateway is unavailable") from exc - return response.json() + payload = response.json() + if response.status_code >= 400: + return JSONResponse(status_code=response.status_code, content=payload) + return payload class InboundSMSPayload(BaseModel): @@ -72,6 +91,46 @@ class InboundSMSPayload(BaseModel): body: str +class GsmFailureDetail(BaseModel): + message: str + reason: str + msg_id: str | None = None + + +class GsmFailureResponse(BaseModel): + detail: GsmFailureDetail + + +class GsmHealthResponse(BaseModel): + status: str + gsm_ready: bool + connected: bool + detail: str + + +class GsmHealthUnavailableResponse(BaseModel): + detail: str + + +GSM_SEND_ERROR_RESPONSES = { + 502: { + "model": GsmFailureResponse, + "description": "The modem rejected the SMS or delivery confirmation timed out.", + }, + 503: { + "model": GsmFailureResponse | GsmHealthUnavailableResponse, + "description": "The GSM gateway is unavailable, stopping, or at queue capacity.", + }, +} + +GSM_HEALTH_ERROR_RESPONSES = { + 503: { + "model": GsmHealthResponse | GsmHealthUnavailableResponse, + "description": "The GSM gateway is unavailable or reports degraded health.", + } +} + + def _gsm_secret_ok(request: Request) -> bool: return request.headers.get("X-GSM-Secret") == GSM_SECRET @@ -202,14 +261,14 @@ async def inbound_sms( return {"ok": True, "message_id": str(msg.id)} -@router.get("/health") +@router.get("/health", responses=GSM_HEALTH_ERROR_RESPONSES) async def gsm_health( current_user : Annotated[User, Depends(get_current_user)], ): return await _get_gsm_health("/health", current_user) -@router.get("/health/detailed") +@router.get("/health/detailed", responses=GSM_HEALTH_ERROR_RESPONSES) async def gsm_health_detailed( current_user : Annotated[User, Depends(get_current_user_admin)], ): @@ -236,7 +295,7 @@ async def gsm_messages( response = await client.get("/sms/messages", params=params) return response.json() -@router.post("/sms/send") +@router.post("/sms/send", responses=GSM_SEND_ERROR_RESPONSES) async def send_sms( current_user : Annotated[User, Depends(get_current_user)], user_id: UUID, @@ -260,13 +319,25 @@ async def send_sms( async def sendToModule(phone_number: str, message: str): client = _get_gsm_client() - response = await client.post( - "/sms/send", - json={"number": phone_number, "body": f"FROM {phone_number}: " + message}, - ) - return response.json() - -@router.post("/request") + try: + response = await client.post( + "/sms/send", + json={"number": phone_number, "body": f"FROM {phone_number}: " + message}, + headers={"X-GSM-Secret": GSM_SECRET}, + ) + except httpx.RequestError as exc: + raise HTTPException(status_code=503, detail={ + "message": "GSM gateway is unavailable", + "reason": "GATEWAY_UNAVAILABLE", + }) from exc + + payload = response.json() + if response.status_code >= 400: + detail = payload.get("detail", payload) if isinstance(payload, dict) else payload + raise HTTPException(status_code=response.status_code, detail=detail) + return payload + +@router.post("/request", responses=GSM_SEND_ERROR_RESPONSES) async def request_phone_verification( data: RequestPhoneVerification, request: Request, @@ -402,7 +473,7 @@ def verify_phone_code( # RESEND CODE # ============================================================================= -@router.post("/resend") +@router.post("/resend", responses=GSM_SEND_ERROR_RESPONSES) async def resend_phone_code( current_user : Annotated[User, Depends(get_current_user)], session: SessionDep @@ -574,7 +645,7 @@ def sms_conversation_id(user_id_a: str, user_id_b: str) -> str: -@router.post("/contact-unknown-user") +@router.post("/contact-unknown-user", responses=GSM_SEND_ERROR_RESPONSES) async def contact_unknown_user( current_user : Annotated[User, Depends(get_current_user)], target_phone_number: Annotated[str, Query(pattern=r"^\+639\d{9}$")], diff --git a/server/app/tests/test_gsm_health.py b/server/app/tests/test_gsm_health.py index 6e05b23a..45d6cf11 100644 --- a/server/app/tests/test_gsm_health.py +++ b/server/app/tests/test_gsm_health.py @@ -12,6 +12,19 @@ async def get(self, path: str): raise httpx.ConnectError("All connection attempts failed") +class DegradedGsmClient: + async def get(self, path: str): + return httpx.Response( + 503, + json={ + "status": "degraded", + "gsm_ready": False, + "connected": True, + "detail": "network unavailable", + }, + ) + + def test_gsm_health_reports_unavailable_gateway(client, monkeypatch, caplog): monkeypatch.setattr(gsm, "_get_gsm_client", lambda: UnavailableGsmClient()) monkeypatch.setitem(app.dependency_overrides, get_current_user, lambda: None) @@ -26,3 +39,18 @@ def test_gsm_health_reports_unavailable_gateway(client, monkeypatch, caplog): assert log_record.user_id == "ANONYMOUS" assert log_record.action == "gsm_health_unavailable" assert log_record.metadata_json == {"path": "/health"} + + +def test_gsm_health_preserves_degraded_gateway_status(client, monkeypatch): + monkeypatch.setattr(gsm, "_get_gsm_client", lambda: DegradedGsmClient()) + monkeypatch.setitem(app.dependency_overrides, get_current_user, lambda: None) + + response = client.get("/gsm/health") + + assert response.status_code == 503 + assert response.json() == { + "status": "degraded", + "gsm_ready": False, + "connected": True, + "detail": "network unavailable", + } diff --git a/server/app/tests/test_gsm_proxy.py b/server/app/tests/test_gsm_proxy.py new file mode 100644 index 00000000..65738769 --- /dev/null +++ b/server/app/tests/test_gsm_proxy.py @@ -0,0 +1,247 @@ +import asyncio + +import httpx + +from app.api import gsm +from app.db_operations.token import get_current_user +from app.main import app +from app.models.phone_verification import PhoneVerification, now_ms +from app.models.users import User +from sqlmodel import select + + +QUEUE_FULL_RESPONSE = { + "detail": { + "message": "Outbound SMS queue is full", + "reason": "QUEUE_FULL", + "msg_id": "sms-log-id", + } +} + +SERVICE_STOPPING_RESPONSE = { + "detail": { + "message": "SMS service is stopping", + "reason": "SERVICE_STOPPING", + "msg_id": "sms-log-id", + } +} + + +class FakeGsmResponse: + def __init__(self, status_code: int, payload: dict): + self.status_code = status_code + self._payload = payload + + @property + def is_error(self) -> bool: + return self.status_code >= 400 + + def json(self) -> dict: + return self._payload + + +class SaturatedGsmClient: + async def post(self, path: str, json: dict, **kwargs): + return FakeGsmResponse(503, QUEUE_FULL_RESPONSE) + + +class StoppingGsmClient: + async def post(self, path: str, json: dict, **kwargs): + return FakeGsmResponse(503, SERVICE_STOPPING_RESPONSE) + + +class UnavailableGsmClient: + async def post(self, path: str, json: dict, **kwargs): + raise httpx.ConnectError("All connection attempts failed") + + +class ModemNotReadyGsmClient: + async def post(self, path: str, json: dict, **kwargs): + return FakeGsmResponse(503, {"detail": "GSM modem not ready"}) + + +class PoolExhaustedGsmClient: + async def post(self, path: str, json: dict, **kwargs): + raise httpx.PoolTimeout("GSM proxy connection pool is full") + + +def _authenticated_user(session): + return session.exec(select(User)).first() + + +def test_proxy_capacity_and_timeouts_cover_gateway_contract(monkeypatch): + captured = {} + + class CapturingClient: + is_closed = False + + def __init__(self, **kwargs): + captured.update(kwargs) + + monkeypatch.setattr(gsm, "_gsm_http_client", None) + monkeypatch.setattr(gsm.httpx, "AsyncClient", CapturingClient) + + gsm._get_gsm_client() + + timeout = captured["timeout"] + limits = captured["limits"] + assert timeout.read == gsm.GSM_PROXY_READ_TIMEOUT_SECONDS + assert timeout.read > gsm.GSM_GATEWAY_WORST_CASE_SECONDS + assert timeout.pool == gsm.GSM_PROXY_POOL_TIMEOUT_SECONDS + assert limits.max_connections == gsm.GSM_PROXY_MAX_CONNECTIONS + assert limits.max_connections > gsm.GSM_GATEWAY_MAX_ADMITTED_REQUESTS + + +def test_send_to_module_authenticates_with_shared_secret(monkeypatch): + captured = {} + + class CapturingClient: + async def post(self, path: str, **kwargs): + captured["path"] = path + captured.update(kwargs) + return FakeGsmResponse(200, {"ok": True}) + + monkeypatch.setattr(gsm, "_get_gsm_client", lambda: CapturingClient()) + + result = asyncio.run(gsm.sendToModule("+639171234567", "message")) + + assert result == {"ok": True} + assert captured["path"] == "/sms/send" + assert captured["headers"] == {"X-GSM-Secret": gsm.GSM_SECRET} + + +def test_send_sms_preserves_queue_full_status(client, session, monkeypatch): + current_user = _authenticated_user(session) + target = session.exec(select(User).where(User.id != current_user.id)).first() + monkeypatch.setattr(gsm, "_get_gsm_client", lambda: SaturatedGsmClient()) + monkeypatch.setitem(app.dependency_overrides, get_current_user, lambda: current_user) + + response = client.post( + "/gsm/sms/send", + params={"user_id": str(target.id), "message": "Help is on the way"}, + ) + + assert response.status_code == 503 + assert response.json() == QUEUE_FULL_RESPONSE + + +def test_send_sms_preserves_service_stopping_status(client, session, monkeypatch): + current_user = _authenticated_user(session) + target = session.exec(select(User).where(User.id != current_user.id)).first() + monkeypatch.setattr(gsm, "_get_gsm_client", lambda: StoppingGsmClient()) + monkeypatch.setitem(app.dependency_overrides, get_current_user, lambda: current_user) + + response = client.post( + "/gsm/sms/send", + params={"user_id": str(target.id), "message": "Help is on the way"}, + ) + + assert response.status_code == 503 + assert response.json() == SERVICE_STOPPING_RESPONSE + + +def test_phone_verification_preserves_queue_full_status(client, session, monkeypatch): + current_user = _authenticated_user(session) + monkeypatch.setattr(gsm, "_get_gsm_client", lambda: SaturatedGsmClient()) + monkeypatch.setitem(app.dependency_overrides, get_current_user, lambda: current_user) + + response = client.post( + "/gsm/request", + json={"phone_number": current_user.phone_number}, + ) + + assert response.status_code == 503 + assert response.json() == QUEUE_FULL_RESPONSE + + +def test_phone_verification_resend_preserves_queue_full_status( + client, session, monkeypatch +): + current_user = _authenticated_user(session) + session.add( + PhoneVerification( + user_id=current_user.id, + phone_number=current_user.phone_number, + verification_code="123456", + expires_at=now_ms() + 300_000, + ) + ) + session.commit() + monkeypatch.setattr(gsm, "_get_gsm_client", lambda: SaturatedGsmClient()) + monkeypatch.setitem(app.dependency_overrides, get_current_user, lambda: current_user) + + response = client.post("/gsm/resend") + + assert response.status_code == 503 + assert response.json() == QUEUE_FULL_RESPONSE + + +def test_contact_unknown_user_preserves_queue_full_status(client, session, monkeypatch): + current_user = _authenticated_user(session) + monkeypatch.setattr(gsm, "_get_gsm_client", lambda: SaturatedGsmClient()) + monkeypatch.setitem(app.dependency_overrides, get_current_user, lambda: current_user) + + response = client.post( + "/gsm/contact-unknown-user", + params={"target_phone_number": "+639991234567"}, + ) + + assert response.status_code == 503 + assert response.json() == QUEUE_FULL_RESPONSE + + +def test_send_sms_reports_unavailable_gateway(client, session, monkeypatch): + current_user = _authenticated_user(session) + target = session.exec(select(User).where(User.id != current_user.id)).first() + monkeypatch.setattr(gsm, "_get_gsm_client", lambda: UnavailableGsmClient()) + monkeypatch.setitem(app.dependency_overrides, get_current_user, lambda: current_user) + + response = client.post( + "/gsm/sms/send", + params={"user_id": str(target.id), "message": "Help is on the way"}, + ) + + assert response.status_code == 503 + assert response.json() == { + "detail": { + "message": "GSM gateway is unavailable", + "reason": "GATEWAY_UNAVAILABLE", + } + } + + +def test_send_sms_preserves_modem_not_ready_status(client, session, monkeypatch): + current_user = _authenticated_user(session) + target = session.exec(select(User).where(User.id != current_user.id)).first() + monkeypatch.setattr(gsm, "_get_gsm_client", lambda: ModemNotReadyGsmClient()) + monkeypatch.setitem(app.dependency_overrides, get_current_user, lambda: current_user) + + response = client.post( + "/gsm/sms/send", + params={"user_id": str(target.id), "message": "Help is on the way"}, + ) + + assert response.status_code == 503 + assert response.json() == {"detail": "GSM modem not ready"} + + +def test_send_sms_rejects_proxy_pool_exhaustion_without_gateway_send( + client, session, monkeypatch +): + current_user = _authenticated_user(session) + target = session.exec(select(User).where(User.id != current_user.id)).first() + monkeypatch.setattr(gsm, "_get_gsm_client", lambda: PoolExhaustedGsmClient()) + monkeypatch.setitem(app.dependency_overrides, get_current_user, lambda: current_user) + + response = client.post( + "/gsm/sms/send", + params={"user_id": str(target.id), "message": "Help is on the way"}, + ) + + assert response.status_code == 503 + assert response.json() == { + "detail": { + "message": "GSM gateway is unavailable", + "reason": "GATEWAY_UNAVAILABLE", + } + } diff --git a/server/nginx.conf b/server/nginx.conf index 371b702d..b43c2227 100644 --- a/server/nginx.conf +++ b/server/nginx.conf @@ -68,7 +68,7 @@ server { proxy_pass http://127.0.0.1:8000; proxy_set_header Host $host; - proxy_read_timeout 135s; + proxy_read_timeout 155s; proxy_set_header Host $host; proxy_set_header X-Real-IP $remote_addr; # proxy_set_header X-Forwarded-For $remote_addr; From 36e17d08c280b62613e299bd9c80054a83e1c514 Mon Sep 17 00:00:00 2001 From: Adamskiee Date: Fri, 14 Aug 2026 14:13:34 +0800 Subject: [PATCH 13/15] fix(mobile-gsm): retain failed SMS for retry --- .../app/(drawer)/(tabs)/chat/[id].tsx | 6 +- .../app/(drawer)/(tabs)/index.tsx | 13 ++- .../settings/account/phone/verify-phone.tsx | 34 +++---- mobile-app/sapot-mobile-app/docs/API.md | 18 ++++ .../sapot-mobile-app/docs/ARCHITECTURE.md | 3 + .../docs/diagrams/05-sms-flow.md | 4 +- .../auth/api/__tests__/auth.api.test.ts | 45 ++++++++- .../features/auth/api/auth.api.ts | 43 ++++----- .../features/auth/auth-container.test.ts | 5 + .../features/auth/auth-container.ts | 3 + .../features/auth/hooks/index.ts | 2 +- .../hooks/use-phone-verification-service.ts | 5 + .../services/phone-verification-service.ts | 24 +++++ .../__tests__/message-list.test.tsx | 86 +++++++++++++++-- .../features/chat/components/message-list.tsx | 31 +++++- .../chat/hooks/use-send-message.test.ts | 46 ++++++++- .../features/chat/hooks/use-send-message.ts | 17 +++- .../main-container-initialize.test.ts | 1 + .../shared/connection/services/gsm-service.ts | 19 ++++ .../shared/connection/services/index.ts | 1 + .../shared/core/api/__tests__/gsm.api.test.ts | 51 ++++++++++ .../features/shared/core/api/gsm.api.ts | 29 ++++-- .../core/errors/__tests__/gsm-error.test.ts | 72 ++++++++++++++ .../features/shared/core/errors/gsm-error.ts | 96 +++++++++++++++++++ .../features/shared/core/errors/index.ts | 7 ++ .../features/shared/hooks/index.ts | 2 +- .../features/shared/hooks/use-gsm-health.ts | 7 +- .../features/shared/hooks/use-gsm-service.ts | 5 + .../features/shared/main-container.ts | 3 + 29 files changed, 595 insertions(+), 83 deletions(-) create mode 100644 mobile-app/sapot-mobile-app/features/auth/hooks/use-phone-verification-service.ts create mode 100644 mobile-app/sapot-mobile-app/features/auth/services/phone-verification-service.ts create mode 100644 mobile-app/sapot-mobile-app/features/shared/connection/services/gsm-service.ts create mode 100644 mobile-app/sapot-mobile-app/features/shared/core/api/__tests__/gsm.api.test.ts create mode 100644 mobile-app/sapot-mobile-app/features/shared/core/errors/__tests__/gsm-error.test.ts create mode 100644 mobile-app/sapot-mobile-app/features/shared/core/errors/gsm-error.ts create mode 100644 mobile-app/sapot-mobile-app/features/shared/hooks/use-gsm-service.ts diff --git a/mobile-app/sapot-mobile-app/app/(drawer)/(tabs)/chat/[id].tsx b/mobile-app/sapot-mobile-app/app/(drawer)/(tabs)/chat/[id].tsx index e9416c3d..873354c6 100644 --- a/mobile-app/sapot-mobile-app/app/(drawer)/(tabs)/chat/[id].tsx +++ b/mobile-app/sapot-mobile-app/app/(drawer)/(tabs)/chat/[id].tsx @@ -661,7 +661,11 @@ const ChatRoom = () => { {conversationId ? ( - + ) : ( No messages yet diff --git a/mobile-app/sapot-mobile-app/app/(drawer)/(tabs)/index.tsx b/mobile-app/sapot-mobile-app/app/(drawer)/(tabs)/index.tsx index 42f34aae..f665ea44 100644 --- a/mobile-app/sapot-mobile-app/app/(drawer)/(tabs)/index.tsx +++ b/mobile-app/sapot-mobile-app/app/(drawer)/(tabs)/index.tsx @@ -6,13 +6,13 @@ import { ChatRoomSource } from "@/features/chat/types"; import { toInternationalPhone } from "@/features/auth/utils/validation"; import { ChatList, useChats } from "@/features/chat"; import { useChatService } from "@/features/chat/hooks/use-chat-service"; -import { contactUnknownUser } from "@/features/shared/core/api/gsm.api"; import { AppSnackbar } from "@/features/shared/components/app-snackbar"; import PeerList from "@/features/shared/components/peer-list"; import { Peer } from "@/features/shared/core/database"; import { useConnectionService, useDiscoveryService, + useGsmService, usePeerService, useToast, } from "@/features/shared/hooks"; @@ -21,6 +21,7 @@ import { useGsmHealth } from "@/features/shared/hooks/use-gsm-health"; import { useSyncService } from "@/features/shared/hooks/use-sync-service"; import { useUserStore } from "@/features/shared/hooks/use-user-store"; import { uiLog } from "@/features/shared/core/utils/logger"; +import { getGsmErrorMessage } from "@/features/shared/core/errors"; import { useHeaderHeight } from "@react-navigation/elements"; import { LinearGradient } from "expo-linear-gradient"; import { useRouter } from "expo-router"; @@ -94,6 +95,7 @@ export default function Chat() { const connectionService = useConnectionService(); const peerService = usePeerService(); const chatService = useChatService(); + const gsmService = useGsmService(); useEffect(() => { uiLog.debug("[Chat] useEffect triggered, deps:", { @@ -252,7 +254,7 @@ export default function Chat() { setContacting(true); try { const phone = toInternationalPhone(targetPhone.trim()); - const res = await contactUnknownUser(phone); + const res = await gsmService.contactUnknownUser(phone); await peerService.upsertPeer({ id: res.user_id, username: phone, @@ -284,9 +286,12 @@ export default function Chat() { source: ChatRoomSource.PEER, }, }); - } catch { + } catch (error) { showError( - "Failed to contact user. Check phone number format (+63...)." + getGsmErrorMessage( + error, + "Failed to contact user. Check phone number format (+63...)." + ) ); } finally { setContacting(false); diff --git a/mobile-app/sapot-mobile-app/app/(drawer)/settings/account/phone/verify-phone.tsx b/mobile-app/sapot-mobile-app/app/(drawer)/settings/account/phone/verify-phone.tsx index 260edbe2..5a07e999 100644 --- a/mobile-app/sapot-mobile-app/app/(drawer)/settings/account/phone/verify-phone.tsx +++ b/mobile-app/sapot-mobile-app/app/(drawer)/settings/account/phone/verify-phone.tsx @@ -1,18 +1,13 @@ import { SETTINGS_ROUTES } from "@/config/routes"; -import { useUserService } from "@/features/auth"; -import { - checkGsmHealth, - migratePhoneUserApi, - requestPhoneVerification, - resendVerificationCodePhone, - verifyCodePhone, -} from "@/features/auth/api/auth.api"; +import { usePhoneVerificationService, useUserService } from "@/features/auth"; import { toInternationalPhone } from "@/features/auth/utils/validation"; import { useRecoveryKeySetup } from "@/features/auth/hooks/use-recovery-key-setup"; import { VerificationCodeContent } from "@/features/settings"; import AppSnackbar from "@/features/shared/components/app-snackbar"; import { useSyncService } from "@/features/shared/hooks/use-sync-service"; +import { useGsmService } from "@/features/shared/hooks"; import { uiLog } from "@/features/shared/core/utils/logger"; +import { getGsmErrorMessage } from "@/features/shared/core/errors"; import { router, useLocalSearchParams } from "expo-router"; import { useEffect, useState } from "react"; import { View } from "react-native"; @@ -35,6 +30,8 @@ export default function VerifyPhone() { variant: "neutral", }); const userService = useUserService(); + const gsmService = useGsmService(); + const phoneVerificationService = usePhoneVerificationService(); const { setupPhoneBlob } = useRecoveryKeySetup(); const syncService = useSyncService(); @@ -50,8 +47,8 @@ export default function VerifyPhone() { const sendCode = async () => { setIsSending(true); setSendFailed(false); - const gsmOnline = await checkGsmHealth(); - if (!gsmOnline) { + const gsmHealth = await gsmService.getHealth().catch(() => null); + if (!gsmHealth?.gsm_ready) { setSnackbar({ visible: true, message: @@ -64,7 +61,7 @@ export default function VerifyPhone() { } try { setIsSending(false); - await requestPhoneVerification( + await phoneVerificationService.requestVerification( phone ? toInternationalPhone(phone) : undefined, reauth_token || undefined ); @@ -76,7 +73,10 @@ export default function VerifyPhone() { setSendFailed(true); setSnackbar({ visible: true, - message: "Failed to send verification code. Please try again.", + message: getGsmErrorMessage( + error, + "Failed to send verification code. Please try again." + ), variant: "error", }); } @@ -91,14 +91,14 @@ export default function VerifyPhone() { setCodeError(undefined); try { - await verifyCodePhone(code); + await phoneVerificationService.verifyCode(code); await userService.updateAuthenticatedUser({ phoneNumber: toInternationalPhone(phone), phoneNumberVerified: true, }); await setupPhoneBlob(phone); try { - const migration = await migratePhoneUserApi(); + const migration = await phoneVerificationService.migratePhoneUser(); if (migration.migrated) { uiLog.info("[VerifyPhone] ghost user migrated", { ghostUserId: migration.ghost_user_id, @@ -127,10 +127,12 @@ export default function VerifyPhone() { setCodeError(undefined); try { - await resendVerificationCodePhone(); + await phoneVerificationService.resendCode(); } catch (error) { uiLog.error("[VerifyPhone] Error resending code", { error }); - setCodeError("Failed to resend code. Please try again."); + setCodeError( + getGsmErrorMessage(error, "Failed to resend code. Please try again.") + ); } }; diff --git a/mobile-app/sapot-mobile-app/docs/API.md b/mobile-app/sapot-mobile-app/docs/API.md index 670b0823..79d59109 100644 --- a/mobile-app/sapot-mobile-app/docs/API.md +++ b/mobile-app/sapot-mobile-app/docs/API.md @@ -848,6 +848,8 @@ directly. { "status": "string", "gsm_ready": "boolean", "connected": "boolean", "detail": "string" } ``` +The server preserves the gateway's HTTP 503 status when the modem is not ready. Callers treat that response as unavailable rather than relying only on the JSON status field. + `gsm_ready` (modem registered on the network) and `connected` (API can reach the GSM service) fail independently — surface them separately rather than collapsing to one "offline" state. @@ -862,6 +864,19 @@ independently — surface them separately rather than collapsing to one "offline { "msg_id": "string", "ok": "boolean", "to": "string" } ``` +**Response `503` when the outbound queue is full:** +```json +{ + "detail": { + "message": "Outbound SMS queue is full", + "reason": "QUEUE_FULL", + "msg_id": "string" + } +} +``` + +The chat screen marks the local message `not_sent`, shows that the SMS service is busy, and keeps the manual resend action available. + --- ### `POST /gsm/contact-unknown-user` — SMS an Arbitrary Number @@ -879,6 +894,7 @@ independently — surface them separately rather than collapsing to one "offline ``` `is_sapot_user` reports whether the number already belongs to a registered account. +If the onboarding SMS is rejected, the endpoint preserves the gateway error and the app does not display its success confirmation. --- @@ -900,6 +916,8 @@ independently — surface them separately rather than collapsing to one "offline { "message": "string" } ``` +Phone verification request and resend calls preserve HTTP 503 gateway failures. The verification screen remains retryable and displays a busy or unavailable message. + --- ### `POST /gsm/migrate-phone-user` — Claim a Ghost Phone Account diff --git a/mobile-app/sapot-mobile-app/docs/ARCHITECTURE.md b/mobile-app/sapot-mobile-app/docs/ARCHITECTURE.md index fade9f60..d6e1ce64 100644 --- a/mobile-app/sapot-mobile-app/docs/ARCHITECTURE.md +++ b/mobile-app/sapot-mobile-app/docs/ARCHITECTURE.md @@ -21,6 +21,7 @@ and takes no arguments: - `guestUserRepository` — guest profile row - `guestMigrationService` — guest→auth conversion - `userService` — login/logout; `MainContainer` injects the `CleanUpService` into it so logout purges local data +- `phoneVerificationService`: phone verification request/resend, code verification, and ghost-user migration. Screens access it through `usePhoneVerificationService()`; GSM availability comes from the shared `GsmService`. ### MainContainer (`features/shared/main-container.ts`) @@ -142,6 +143,8 @@ Crypto stack: `tweetnacl` + `tweetnacl-util`, `@noble/hashes`, `expo-crypto`, `r | `NotificationService` | Local incoming-call notifications via `expo-notifications`. Constructed inline in `main-container.ts` and passed to `ConnectionService`; not exposed as a container field. | | `CallMessageRouter` | Pure decision layer for inbound call messages. Maps a `CallMessage` + busy/active state to a `CallRouterResult` (`emit` / suppress), keeping glare handling out of `ConnectionService`. | | `PublicChatService` | Server-relayed public chat over `WsSignalingAdapter`, with history loaded from `GET /public-chat`. Independent of the P2P chat path. | +| `GsmService` | Owned by `MainContainer`. Reads GSM health, sends chat SMS, and sends first-contact onboarding requests through the GSM API. UI and chat hooks access it through `useGsmService()` so screens do not call API modules directly. | +| `PhoneVerificationService` | Owned by `AuthContainer`. Coordinates phone verification request/resend, code verification, and ghost-user migration through the auth API module. GSM health remains centralized in `GsmService`. | --- diff --git a/mobile-app/sapot-mobile-app/docs/diagrams/05-sms-flow.md b/mobile-app/sapot-mobile-app/docs/diagrams/05-sms-flow.md index bb2822cf..6df903d2 100644 --- a/mobile-app/sapot-mobile-app/docs/diagrams/05-sms-flow.md +++ b/mobile-app/sapot-mobile-app/docs/diagrams/05-sms-flow.md @@ -16,7 +16,9 @@ flowchart TD F --> Z([End]) E -->|Yes| G[Forward message to server] G --> H[Server sends message to GSM module] - H --> I[GSM module transmits SMS over cellular network] + H --> Q{Outbound queue has capacity?} + Q -->|No| K[Keep message as not sent and display busy error] + Q -->|Yes| I[GSM module transmits SMS over cellular network] I --> J{Transmission successful?} J -->|No| K[Display send failure] K --> Z diff --git a/mobile-app/sapot-mobile-app/features/auth/api/__tests__/auth.api.test.ts b/mobile-app/sapot-mobile-app/features/auth/api/__tests__/auth.api.test.ts index 8a96327f..8fca017b 100644 --- a/mobile-app/sapot-mobile-app/features/auth/api/__tests__/auth.api.test.ts +++ b/mobile-app/sapot-mobile-app/features/auth/api/__tests__/auth.api.test.ts @@ -1,4 +1,10 @@ -import { loginAsFixtureApi, resetPasswordApi } from "../auth.api"; +import { GsmGatewayError } from "@/features/shared/core/errors/gsm-error"; +import { + loginAsFixtureApi, + requestPhoneVerification, + resendVerificationCodePhone, + resetPasswordApi, +} from "../auth.api"; jest.mock("@/features/shared", () => ({ apiClient: { post: jest.fn(), get: jest.fn() }, @@ -56,3 +62,40 @@ describe("loginAsFixtureApi", () => { ); }); }); + +describe("GSM verification API", () => { + const queueFullError = { + response: { + status: 503, + data: { + detail: { + message: "Outbound SMS queue is full", + reason: "QUEUE_FULL", + msg_id: "sms-log-id", + }, + }, + }, + }; + + it("wraps verification saturation as a typed GSM gateway error", async () => { + mockPost.mockRejectedValue(queueFullError); + + await expect( + requestPhoneVerification("+639171234567") + ).rejects.toMatchObject({ + name: "GsmGatewayError", + status: 503, + reason: "QUEUE_FULL", + } satisfies Partial); + }); + + it("wraps resend saturation as a typed GSM gateway error", async () => { + mockPost.mockRejectedValue(queueFullError); + + await expect(resendVerificationCodePhone()).rejects.toMatchObject({ + name: "GsmGatewayError", + status: 503, + reason: "QUEUE_FULL", + } satisfies Partial); + }); +}); diff --git a/mobile-app/sapot-mobile-app/features/auth/api/auth.api.ts b/mobile-app/sapot-mobile-app/features/auth/api/auth.api.ts index 9fa0c3c0..f2d7c773 100644 --- a/mobile-app/sapot-mobile-app/features/auth/api/auth.api.ts +++ b/mobile-app/sapot-mobile-app/features/auth/api/auth.api.ts @@ -1,5 +1,5 @@ import { QA_API_TOKEN } from "@/config/debug"; -import { toAppError } from "@/features/shared/core/errors"; +import { toAppError, toGsmGatewayError } from "@/features/shared/core/errors"; import { apiClient } from "@/features/shared"; import { apiLog } from "@/features/shared/core/utils/logger"; import { AxiosResponse } from "axios"; @@ -327,32 +327,23 @@ export const verifyCodeEmail = async (code: string) => { return res.data; }; -export const checkGsmHealth = async (): Promise => { - try { - apiLog.info("[AuthApi] Calling /gsm/health"); - const res = await apiClient.get<{ status: string }>("/gsm/health"); - apiLog.info("[AuthApi] GSM health response", { status: res.status }); - return res.status === 200; - } catch (error) { - const appErr = toAppError(error, "auth"); - apiLog.warn("[AuthApi] GSM health check failed", appErr); - return false; - } -}; - export const requestPhoneVerification = async ( phoneNumber?: string, reauthToken?: string ) => { apiLog.info("[AuthApi] Calling /gsm/request", { hasPhoneNumber: Boolean(phoneNumber) }); - const res = await apiClient.post<{ message: string }>( - "/gsm/request", - { phone_number: phoneNumber }, - reauthToken ? { headers: { "X-Reauth-Token": reauthToken } } : undefined - ); + try { + const res = await apiClient.post<{ message: string }>( + "/gsm/request", + { phone_number: phoneNumber }, + reauthToken ? { headers: { "X-Reauth-Token": reauthToken } } : undefined + ); - apiLog.info("[AuthApi] Response received", { status: res.status }); - return res.data; + apiLog.info("[AuthApi] Response received", { status: res.status }); + return res.data; + } catch (error) { + throw toGsmGatewayError(error); + } }; export const verifyCodePhone = async (code: string) => { @@ -396,10 +387,14 @@ export const fetchTermsContent = async (): Promise => { export const resendVerificationCodePhone = async () => { apiLog.info("[AuthApi] Calling /gsm/resend"); - const res = await apiClient.post<{ message: string }>("/gsm/resend"); + try { + const res = await apiClient.post<{ message: string }>("/gsm/resend"); - apiLog.info("[AuthApi] Response received", { status: res.status }); - return res.data; + apiLog.info("[AuthApi] Response received", { status: res.status }); + return res.data; + } catch (error) { + throw toGsmGatewayError(error); + } }; export const migratePhoneUserApi = async (): Promise<{ diff --git a/mobile-app/sapot-mobile-app/features/auth/auth-container.test.ts b/mobile-app/sapot-mobile-app/features/auth/auth-container.test.ts index 128941bf..6afc3408 100644 --- a/mobile-app/sapot-mobile-app/features/auth/auth-container.test.ts +++ b/mobile-app/sapot-mobile-app/features/auth/auth-container.test.ts @@ -20,6 +20,9 @@ jest.mock("../shared/peer/guest-user-repository", () => ({ jest.mock("../shared/connection/services/user-service", () => ({ UserService: jest.fn().mockImplementation(() => ({ initialize: jest.fn() })), })); +jest.mock("./services/phone-verification-service", () => ({ + PhoneVerificationService: jest.fn().mockImplementation(() => ({})), +})); describe("AuthContainer", () => { it("constructs dependencies", () => { @@ -34,6 +37,8 @@ describe("AuthContainer", () => { expect(shared.UserStore).toHaveBeenCalledTimes(1); expect(shared.GuestUserRepository).toHaveBeenCalledWith(shared.database); expect(shared.UserService).toHaveBeenCalledTimes(1); + const { PhoneVerificationService } = require("./services/phone-verification-service"); + expect(PhoneVerificationService).toHaveBeenCalledTimes(1); expect(container).toBeInstanceOf(AuthContainer); }); diff --git a/mobile-app/sapot-mobile-app/features/auth/auth-container.ts b/mobile-app/sapot-mobile-app/features/auth/auth-container.ts index 2452cd7d..68dc1568 100644 --- a/mobile-app/sapot-mobile-app/features/auth/auth-container.ts +++ b/mobile-app/sapot-mobile-app/features/auth/auth-container.ts @@ -8,6 +8,7 @@ import { SessionStore } from "../shared/core/stores/session-store"; import { UserStore } from "../shared/core/stores/user-store"; import { authLog } from "../shared/core/utils/logger"; import { GuestMigrationService } from "./services/guest-migration-service"; +import { PhoneVerificationService } from "./services/phone-verification-service"; authLog.debug("[auth-container] module loaded"); @@ -17,6 +18,7 @@ export class AuthContainer { readonly peerRepository: PeerRepository; readonly guestUserRepository: GuestUserRepository; readonly guestMigrationService: GuestMigrationService; + readonly phoneVerificationService: PhoneVerificationService; readonly userStore: UserStore; readonly sessionStore: SessionStore; private initPromise?: Promise; @@ -33,6 +35,7 @@ export class AuthContainer { this.guestMigrationService = new GuestMigrationService( this.guestUserRepository ); + this.phoneVerificationService = new PhoneVerificationService(); this.userService = new UserService( this.userStore, diff --git a/mobile-app/sapot-mobile-app/features/auth/hooks/index.ts b/mobile-app/sapot-mobile-app/features/auth/hooks/index.ts index f687661d..9c506750 100644 --- a/mobile-app/sapot-mobile-app/features/auth/hooks/index.ts +++ b/mobile-app/sapot-mobile-app/features/auth/hooks/index.ts @@ -13,4 +13,4 @@ export * from "./use-validate-identifier"; export * from "./use-verify-question"; export * from "./use-verify-recovery-key"; export * from "./use-password-reset-key-recovery"; - +export * from "./use-phone-verification-service"; diff --git a/mobile-app/sapot-mobile-app/features/auth/hooks/use-phone-verification-service.ts b/mobile-app/sapot-mobile-app/features/auth/hooks/use-phone-verification-service.ts new file mode 100644 index 00000000..9b8e22c5 --- /dev/null +++ b/mobile-app/sapot-mobile-app/features/auth/hooks/use-phone-verification-service.ts @@ -0,0 +1,5 @@ +import { useAuthContainer } from "./use-auth-container"; + +export function usePhoneVerificationService() { + return useAuthContainer().phoneVerificationService; +} diff --git a/mobile-app/sapot-mobile-app/features/auth/services/phone-verification-service.ts b/mobile-app/sapot-mobile-app/features/auth/services/phone-verification-service.ts new file mode 100644 index 00000000..bdb44464 --- /dev/null +++ b/mobile-app/sapot-mobile-app/features/auth/services/phone-verification-service.ts @@ -0,0 +1,24 @@ +import { + migratePhoneUserApi, + requestPhoneVerification, + resendVerificationCodePhone, + verifyCodePhone, +} from "@/features/auth/api/auth.api"; + +export class PhoneVerificationService { + requestVerification(phone?: string, reauthToken?: string) { + return requestPhoneVerification(phone, reauthToken); + } + + verifyCode(code: string) { + return verifyCodePhone(code); + } + + resendCode() { + return resendVerificationCodePhone(); + } + + migratePhoneUser() { + return migratePhoneUserApi(); + } +} diff --git a/mobile-app/sapot-mobile-app/features/chat/components/__tests__/message-list.test.tsx b/mobile-app/sapot-mobile-app/features/chat/components/__tests__/message-list.test.tsx index 75b100e2..4431eead 100644 --- a/mobile-app/sapot-mobile-app/features/chat/components/__tests__/message-list.test.tsx +++ b/mobile-app/sapot-mobile-app/features/chat/components/__tests__/message-list.test.tsx @@ -1,6 +1,11 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ -import { render } from "@testing-library/react-native"; +import { fireEvent, render, waitFor } from "@testing-library/react-native"; import React from "react"; +import { GsmGatewayError } from "@/features/shared/core/errors/gsm-error"; + +const mockTryResendMessage = jest.fn(); +const mockUpdateMessageStatus = jest.fn(); +const mockSendSmsToUser = jest.fn(); // Mock withObservables to synchronously unwrap simple observables returned // by the mapping function so the enhanced components receive plain values. @@ -31,7 +36,8 @@ jest.mock("@nozbe/watermelondb/react", () => { jest.mock("@/features/chat/hooks/use-chat-service", () => ({ useChatService: () => ({ - tryResendMessage: jest.fn(), + tryResendMessage: mockTryResendMessage, + updateMessageStatus: mockUpdateMessageStatus, }), })); @@ -49,6 +55,9 @@ jest.mock("@/features/shared/hooks", () => ({ }, }), useReducedMotion: () => false, + useGsmService: () => ({ + sendSmsToUser: mockSendSmsToUser, + }), })); jest.mock("@/features/shared/hooks/use-user-store", () => ({ @@ -68,10 +77,6 @@ jest.mock("@/features/call", () => ({ useInformCall: () => jest.fn(), })); -jest.mock("@/features/shared/core/api/gsm.api", () => ({ - sendSmsToUser: jest.fn().mockResolvedValue({ ok: true }), -})); - // Track query args so tests can assert the newest-first + pagination fix // for issue #142 (chat only showed the oldest 100 messages). const messagesQueryCalls: unknown[][] = []; @@ -103,6 +108,16 @@ jest.mock("@/features/shared", () => { // Newest-first order, matching a `Q.sortBy("created_at", Q.desc)` query. const messagesNewestFirst = [ + { + ...makeMessage("sms-retry", "2024-01-03T00:00:00Z"), + messageType: "sms", + content: "Retry SMS", + sender: { + id: "current-user", + observe: () => of({ username: "Current user" }), + }, + _raw: { sender: "current-user" }, + }, makeMessage("msg-2", "2024-01-02T00:00:00Z"), makeMessage("msg-1", "2024-01-01T00:00:00Z"), ]; @@ -117,7 +132,7 @@ jest.mock("@/features/shared", () => { // Return RxJS observables for observes so the // withObservables HOC receives expected observable inputs. observe: () => (table === "messages" ? of(messagesNewestFirst) : of([])), - observeWithColumns: () => of([{ status: "sent" }]), + observeWithColumns: () => of([{ status: "not_sent" }]), }; }, }; @@ -144,11 +159,18 @@ describe("MessageList", () => { beforeEach(() => { messagesQueryCalls.length = 0; + jest.clearAllMocks(); + mockUpdateMessageStatus.mockResolvedValue(undefined); + mockSendSmsToUser.mockResolvedValue({ ok: true }); }); it("renders messages", async () => { const { findByText } = render( - + ); expect(await findByText(/Content msg-2/)).toBeTruthy(); @@ -158,7 +180,11 @@ describe("MessageList", () => { const { Q } = require("@nozbe/watermelondb"); const { findByText } = render( - + ); await findByText(/Content msg-2/); @@ -169,7 +195,11 @@ describe("MessageList", () => { it("opens a conversation on the newest message", async () => { const { findByText, getAllByText } = render( - + ); await findByText(/Content msg-2/); @@ -178,4 +208,40 @@ describe("MessageList", () => { expect(rendered[0].props.children).toContain("msg-2"); expect(rendered[1].props.children).toContain("msg-1"); }); + + it("shows the queue-full error when manual SMS resend is rejected", async () => { + mockSendSmsToUser.mockRejectedValue( + new GsmGatewayError({ + status: 503, + reason: "QUEUE_FULL", + message: "Outbound SMS queue is full", + }) + ); + const showError = jest.fn(); + const { findByText } = render( + + ); + + fireEvent.press(await findByText("Resend")); + + await waitFor(() => { + expect(showError).toHaveBeenCalledWith( + "SMS service is busy. Please try again shortly." + ); + }); + expect(mockUpdateMessageStatus).toHaveBeenNthCalledWith( + 1, + "sms-retry", + "sending" + ); + expect(mockUpdateMessageStatus).toHaveBeenNthCalledWith( + 2, + "sms-retry", + "not_sent" + ); + }); }); diff --git a/mobile-app/sapot-mobile-app/features/chat/components/message-list.tsx b/mobile-app/sapot-mobile-app/features/chat/components/message-list.tsx index 1a757e1c..c396225f 100644 --- a/mobile-app/sapot-mobile-app/features/chat/components/message-list.tsx +++ b/mobile-app/sapot-mobile-app/features/chat/components/message-list.tsx @@ -24,7 +24,7 @@ import { } from "@/features/shared"; import { MessageType } from "@/features/shared/core/database/model/Message"; import { CallType } from "@/features/shared/core/database/model/Call"; -import { useMainContainer, useReducedMotion } from "@/features/shared/hooks"; +import { useGsmService, useMainContainer, useReducedMotion } from "@/features/shared/hooks"; import { ECDH_PREFIX } from "@/features/chat/repositories/message-repository"; import { useUserStore } from "@/features/shared/hooks/use-user-store"; import { MessageStatusType } from "@/features/shared/core/database/model/MessageStatus"; @@ -33,7 +33,7 @@ import { uiLog } from "@/features/shared/core/utils/logger"; import { useChatService } from "@/features/chat/hooks/use-chat-service"; import { useTheme } from "react-native-paper"; import { useInformCall } from "@/features/call"; -import { sendSmsToUser } from "@/features/shared/core/api/gsm.api"; +import { getGsmErrorMessage } from "@/features/shared/core/errors"; import { toLocalPhone } from "@/features/auth/utils/validation"; uiLog.debug("[message-list] module loaded"); @@ -63,10 +63,12 @@ const MessageListWithData = enhanceMessages( ({ messages, peerId, + showError, onLoadOlderMessages, }: { messages: Message[]; peerId: string; + showError: (message: string) => void; onLoadOlderMessages: () => void; }) => { const hasUserScrolledRef = useRef(false); @@ -91,7 +93,13 @@ const MessageListWithData = enhanceMessages( seenIdsRef.current!.add(item.id); if (reducedMotion || !isNewMessage) { - return ; + return ( + + ); } return ( @@ -100,7 +108,11 @@ const MessageListWithData = enhanceMessages( Easing.bezier(...motion.easing.standard) )} > - + ); }} @@ -129,9 +141,11 @@ const MessageListWithData = enhanceMessages( const MessageList = ({ conversationId, peerId, + showError, }: { conversationId: string; peerId: string; + showError: (message: string) => void; }) => { const [messageLimit, setMessageLimit] = useState(MESSAGE_PAGE_SIZE); @@ -148,6 +162,7 @@ const MessageList = ({ key={conversationId} conversationId={conversationId} peerId={peerId} + showError={showError} messageLimit={messageLimit} onLoadOlderMessages={handleLoadOlderMessages} /> @@ -354,6 +369,7 @@ type MessageListItemProps = { guestSender?: GuestUser | null; status: MessageStatus[]; peerId: string; + showError: (message: string) => void; }; const useDecryptedContent = (message: Message): string => { @@ -379,6 +395,7 @@ const MessageListItemInner = memo( guestSender, status, peerId, + showError, }: MessageListItemProps) => { const statusObj = status?.[0]; const senderName = getSenderName(sender ?? guestSender); @@ -387,6 +404,7 @@ const MessageListItemInner = memo( const theme = useTheme(); const isCurrentUserMessage = message.sender?.id === userStore.user?.id; const chatService = useChatService(); + const gsmService = useGsmService(); const peerService = usePeerService(); const [isResending, setIsResending] = useState(false); const { callRepository } = useMainContainer(); @@ -412,7 +430,7 @@ const MessageListItemInner = memo( try { if (message.messageType === MessageType.SMS) { await chatService.updateMessageStatus(message.id, MessageStatusType.SENDING); - const res = await sendSmsToUser(peerId, content); + const res = await gsmService.sendSmsToUser(peerId, content); const status = res.ok ? MessageStatusType.DELIVERED : MessageStatusType.NOT_SENT; @@ -431,6 +449,9 @@ const MessageListItemInner = memo( uiLog.warn("[message-list] resend failed", { peerId, err }); if (message.messageType === MessageType.SMS) { await chatService.updateMessageStatus(message.id, MessageStatusType.NOT_SENT).catch((error) => uiLog.warn("[message-list] reset message status failed", { error })); + showError( + getGsmErrorMessage(err, "SMS could not be delivered. Please try again.") + ); } } finally { setIsResending(false); diff --git a/mobile-app/sapot-mobile-app/features/chat/hooks/use-send-message.test.ts b/mobile-app/sapot-mobile-app/features/chat/hooks/use-send-message.test.ts index 462dcc67..4c191cf4 100644 --- a/mobile-app/sapot-mobile-app/features/chat/hooks/use-send-message.test.ts +++ b/mobile-app/sapot-mobile-app/features/chat/hooks/use-send-message.test.ts @@ -1,8 +1,11 @@ import { renderHook, act } from "@testing-library/react-native"; +import { GsmGatewayError } from "@/features/shared/core/errors/gsm-error"; import { useSendMessage } from "./use-send-message"; -jest.mock("@/features/shared/core/api/gsm.api", () => ({ - sendSmsToUser: jest.fn().mockResolvedValue({ ok: true }), +const mockSendSmsToUser = jest.fn(); + +jest.mock("@/features/shared/hooks", () => ({ + useGsmService: () => ({ sendSmsToUser: mockSendSmsToUser }), })); jest.mock("@/features/shared/core/utils/logger", () => ({ @@ -44,6 +47,15 @@ function makeParams(overrides: Record = {}) { } describe("useSendMessage", () => { + beforeEach(() => { + jest.clearAllMocks(); + mockSendSmsToUser.mockResolvedValue({ + ok: true, + msg_id: "sms-log-id", + to: "+639171234567", + }); + }); + it("returns synchronously before sendChatMessage resolves", () => { let resolveDeferred!: (v: { conversationId: string; messageId: string }) => void; const deferred = new Promise<{ conversationId: string; messageId: string }>( @@ -137,4 +149,34 @@ describe("useSendMessage", () => { expect(params.showError).toHaveBeenCalledWith("Failed to send message"); }); + + it("shows a busy message and marks SMS not sent when the queue is full", async () => { + mockSendSmsToUser.mockRejectedValue( + new GsmGatewayError({ + status: 503, + reason: "QUEUE_FULL", + message: "Outbound SMS queue is full", + messageId: "sms-log-id", + }) + ); + const params = makeParams({ isSmsMode: true }); + const { result } = renderHook(() => useSendMessage(params)); + + act(() => { + result.current(); + }); + + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + }); + + expect(params.chatService.updateMessageStatus).toHaveBeenCalledWith( + "sms-1", + "not_sent" + ); + expect(params.showError).toHaveBeenCalledWith( + "SMS service is busy. Please try again shortly." + ); + }); }); diff --git a/mobile-app/sapot-mobile-app/features/chat/hooks/use-send-message.ts b/mobile-app/sapot-mobile-app/features/chat/hooks/use-send-message.ts index 02fdac21..96fdce03 100644 --- a/mobile-app/sapot-mobile-app/features/chat/hooks/use-send-message.ts +++ b/mobile-app/sapot-mobile-app/features/chat/hooks/use-send-message.ts @@ -1,8 +1,9 @@ import { useCallback } from "react"; -import { sendSmsToUser } from "@/features/shared/core/api/gsm.api"; import { MessageStatusType } from "@/features/shared/core/database/model/MessageStatus"; +import { getGsmErrorMessage } from "@/features/shared/core/errors"; import { uiLog } from "@/features/shared/core/utils/logger"; import { ChatService } from "@/features/chat/services/chat-service"; +import { useGsmService } from "@/features/shared/hooks"; type Params = { message: string; @@ -27,6 +28,8 @@ export function useSendMessage({ peerId, showError, }: Params): () => void { + const gsmService = useGsmService(); + return useCallback(() => { const textToSend = message.trim(); if (!textToSend) return; @@ -42,7 +45,7 @@ export function useSendMessage({ void chatService .sendSmsChannelMessage(textToSend) .then(({ messageId: smsMessageId }) => { - sendSmsToUser(peerId, textToSend) + gsmService.sendSmsToUser(peerId, textToSend) .then((res) => { const status = res.ok ? MessageStatusType.DELIVERED @@ -50,11 +53,16 @@ export function useSendMessage({ chatService.updateMessageStatus(smsMessageId, status).catch((error) => uiLog.warn("use-send-message › SMS status update failed", { error })); if (!res.ok) showError("SMS could not be delivered"); }) - .catch(() => { + .catch((error) => { chatService .updateMessageStatus(smsMessageId, MessageStatusType.NOT_SENT) .catch((error) => uiLog.warn("use-send-message › SMS not_sent status update failed", { error })); - showError("Message sent, but SMS delivery failed."); + showError( + getGsmErrorMessage( + error, + "Message sent, but SMS delivery failed." + ) + ); }); }) .catch((error) => { @@ -85,5 +93,6 @@ export function useSendMessage({ isSmsConversation, peerId, showError, + gsmService, ]); } diff --git a/mobile-app/sapot-mobile-app/features/shared/__tests__/main-container-initialize.test.ts b/mobile-app/sapot-mobile-app/features/shared/__tests__/main-container-initialize.test.ts index cbed3f1f..94cc3a1b 100644 --- a/mobile-app/sapot-mobile-app/features/shared/__tests__/main-container-initialize.test.ts +++ b/mobile-app/sapot-mobile-app/features/shared/__tests__/main-container-initialize.test.ts @@ -97,6 +97,7 @@ jest.mock("../connection/services", () => ({ republish: jest.fn().mockResolvedValue(undefined), destroy: jest.fn().mockResolvedValue(undefined), })), + GsmService: jest.fn().mockImplementation(() => ({})), SignalingService: jest.fn().mockImplementation(() => ({})), WebrtcSessionManager: jest.fn().mockImplementation(() => ({ getWebrtcAdapter: jest.fn(), diff --git a/mobile-app/sapot-mobile-app/features/shared/connection/services/gsm-service.ts b/mobile-app/sapot-mobile-app/features/shared/connection/services/gsm-service.ts new file mode 100644 index 00000000..b194df05 --- /dev/null +++ b/mobile-app/sapot-mobile-app/features/shared/connection/services/gsm-service.ts @@ -0,0 +1,19 @@ +import { + contactUnknownUser, + getGsmHealth, + sendSmsToUser, +} from "@/features/shared/core/api/gsm.api"; + +export class GsmService { + getHealth() { + return getGsmHealth(); + } + + sendSmsToUser(userId: string, message: string) { + return sendSmsToUser(userId, message); + } + + contactUnknownUser(targetPhoneNumber: string) { + return contactUnknownUser(targetPhoneNumber); + } +} diff --git a/mobile-app/sapot-mobile-app/features/shared/connection/services/index.ts b/mobile-app/sapot-mobile-app/features/shared/connection/services/index.ts index 0ced7d7b..cc9a1c0b 100644 --- a/mobile-app/sapot-mobile-app/features/shared/connection/services/index.ts +++ b/mobile-app/sapot-mobile-app/features/shared/connection/services/index.ts @@ -6,6 +6,7 @@ export { CallMediaService } from "./call-media-service"; export * from "./clean-up-service"; export { ConnectionService } from "./connection-service"; export { DiscoveryService } from "./discovery-service"; +export { GsmService } from "./gsm-service"; export { NotificationService } from "./notification-service"; export * from "./service-interfaces"; export { SignalingService } from "./signaling-service"; diff --git a/mobile-app/sapot-mobile-app/features/shared/core/api/__tests__/gsm.api.test.ts b/mobile-app/sapot-mobile-app/features/shared/core/api/__tests__/gsm.api.test.ts new file mode 100644 index 00000000..9b8ecf10 --- /dev/null +++ b/mobile-app/sapot-mobile-app/features/shared/core/api/__tests__/gsm.api.test.ts @@ -0,0 +1,51 @@ +import { GsmGatewayError } from "../../errors/gsm-error"; +import { apiClient } from "../client"; +import { contactUnknownUser, sendSmsToUser } from "../gsm.api"; + +jest.mock("../client", () => ({ + apiClient: { + post: jest.fn(), + }, +})); + +const mockedApiClient = apiClient as jest.Mocked; + +const queueFullError = { + response: { + status: 503, + data: { + detail: { + message: "Outbound SMS queue is full", + reason: "QUEUE_FULL", + msg_id: "sms-log-id", + }, + }, + }, +}; + +describe("GSM API", () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it("wraps send saturation as a typed GSM gateway error", async () => { + mockedApiClient.post.mockRejectedValue(queueFullError); + + await expect(sendSmsToUser("user-id", "message")).rejects.toMatchObject({ + name: "GsmGatewayError", + status: 503, + reason: "QUEUE_FULL", + messageId: "sms-log-id", + } satisfies Partial); + }); + + it("wraps first-contact saturation as a typed GSM gateway error", async () => { + mockedApiClient.post.mockRejectedValue(queueFullError); + + await expect(contactUnknownUser("+639171234567")).rejects.toMatchObject({ + name: "GsmGatewayError", + status: 503, + reason: "QUEUE_FULL", + } satisfies Partial); + }); +}); diff --git a/mobile-app/sapot-mobile-app/features/shared/core/api/gsm.api.ts b/mobile-app/sapot-mobile-app/features/shared/core/api/gsm.api.ts index 3688f5f9..5b2faa7f 100644 --- a/mobile-app/sapot-mobile-app/features/shared/core/api/gsm.api.ts +++ b/mobile-app/sapot-mobile-app/features/shared/core/api/gsm.api.ts @@ -1,4 +1,5 @@ import { apiClient } from "@/features/shared/core/api/client"; +import { toGsmGatewayError } from "@/features/shared/core/errors"; import { apiLog } from "@/features/shared/core/utils/logger"; export type GsmHealthResponse = { @@ -32,20 +33,28 @@ export const sendSmsToUser = async ( message: string ): Promise => { apiLog.debug("api › gsm sms send", { userId }); - const res = await apiClient.post("/gsm/sms/send", null, { - params: { user_id: userId, message }, - }); - return res.data; + try { + const res = await apiClient.post("/gsm/sms/send", null, { + params: { user_id: userId, message }, + }); + return res.data; + } catch (error) { + throw toGsmGatewayError(error); + } }; export const contactUnknownUser = async ( targetPhoneNumber: string ): Promise => { apiLog.debug("api › gsm contact unknown user", { targetPhoneNumber }); - const res = await apiClient.post( - "/gsm/contact-unknown-user", - null, - { params: { target_phone_number: targetPhoneNumber } } - ); - return res.data; + try { + const res = await apiClient.post( + "/gsm/contact-unknown-user", + null, + { params: { target_phone_number: targetPhoneNumber } } + ); + return res.data; + } catch (error) { + throw toGsmGatewayError(error); + } }; diff --git a/mobile-app/sapot-mobile-app/features/shared/core/errors/__tests__/gsm-error.test.ts b/mobile-app/sapot-mobile-app/features/shared/core/errors/__tests__/gsm-error.test.ts new file mode 100644 index 00000000..f2b1e5d8 --- /dev/null +++ b/mobile-app/sapot-mobile-app/features/shared/core/errors/__tests__/gsm-error.test.ts @@ -0,0 +1,72 @@ +import { getGsmErrorMessage, getGsmFailure } from "../gsm-error"; + +describe("GSM gateway errors", () => { + it("recognizes a queue saturation response", () => { + const error = { + response: { + status: 503, + data: { + detail: { + message: "Outbound SMS queue is full", + reason: "QUEUE_FULL", + msg_id: "sms-log-id", + }, + }, + }, + }; + + expect(getGsmFailure(error)).toEqual({ + status: 503, + reason: "QUEUE_FULL", + message: "Outbound SMS queue is full", + messageId: "sms-log-id", + }); + expect(getGsmErrorMessage(error, "fallback")).toBe( + "SMS service is busy. Please try again shortly." + ); + }); + + it("recognizes a service shutdown response", () => { + const error = { + response: { + status: 503, + data: { + detail: { + message: "SMS service is stopping", + reason: "SERVICE_STOPPING", + msg_id: "sms-log-id", + }, + }, + }, + }; + + expect(getGsmErrorMessage(error, "fallback")).toBe( + "SMS service is restarting. Please try again shortly." + ); + }); + + it("recognizes an unavailable modem response", () => { + const error = { + response: { + status: 503, + data: { detail: "GSM modem not ready" }, + }, + }; + + expect(getGsmFailure(error)).toEqual({ + status: 503, + reason: "GATEWAY_UNAVAILABLE", + message: "GSM modem not ready", + }); + expect(getGsmErrorMessage(error, "fallback")).toBe( + "SMS service is unavailable. Please try again later." + ); + }); + + it("uses the supplied fallback for unrelated errors", () => { + expect(getGsmFailure(new Error("network failed"))).toBeUndefined(); + expect(getGsmErrorMessage(new Error("network failed"), "fallback")).toBe( + "fallback" + ); + }); +}); diff --git a/mobile-app/sapot-mobile-app/features/shared/core/errors/gsm-error.ts b/mobile-app/sapot-mobile-app/features/shared/core/errors/gsm-error.ts new file mode 100644 index 00000000..ede2798d --- /dev/null +++ b/mobile-app/sapot-mobile-app/features/shared/core/errors/gsm-error.ts @@ -0,0 +1,96 @@ +import { AppError } from "./app-error"; +import { toAppError } from "./to-app-error"; + +export interface GsmFailure { + status: number; + reason: string; + message: string; + messageId?: string; +} + +export class GsmGatewayError extends AppError { + readonly status: number; + readonly reason: string; + readonly messageId?: string; + + constructor(failure: GsmFailure, cause?: unknown) { + super(failure.message, "network", "medium", cause); + this.name = "GsmGatewayError"; + this.status = failure.status; + this.reason = failure.reason; + this.messageId = failure.messageId; + } +} + +function asRecord(value: unknown): Record | undefined { + return typeof value === "object" && value !== null + ? (value as Record) + : undefined; +} + +export function getGsmFailure(error: unknown): GsmFailure | undefined { + if (error instanceof GsmGatewayError) { + return { + status: error.status, + reason: error.reason, + message: error.message, + messageId: error.messageId, + }; + } + + const response = asRecord(asRecord(error)?.response); + const status = response?.status; + const data = asRecord(response?.data); + const detail = data?.detail; + const detailRecord = asRecord(detail); + + if (typeof status !== "number") return undefined; + + if (detailRecord) { + const reason = detailRecord.reason; + const message = detailRecord.message; + const messageId = detailRecord.msg_id; + if (typeof reason !== "string" || typeof message !== "string") { + return undefined; + } + return { + status, + reason, + message, + messageId: typeof messageId === "string" ? messageId : undefined, + }; + } + + if (status === 503 && typeof detail === "string") { + return { + status, + reason: "GATEWAY_UNAVAILABLE", + message: detail, + }; + } + + return undefined; +} + +export function toGsmGatewayError(error: unknown): AppError { + const failure = getGsmFailure(error); + return failure + ? new GsmGatewayError(failure, error) + : toAppError(error, "network"); +} + +export function getGsmErrorMessage(error: unknown, fallback: string): string { + const failure = getGsmFailure(error); + if (!failure) return fallback; + + if (failure.reason === "QUEUE_FULL") { + return "SMS service is busy. Please try again shortly."; + } + if (failure.reason === "SERVICE_STOPPING") { + return "SMS service is restarting. Please try again shortly."; + } + if (failure.status === 503) { + return "SMS service is unavailable. Please try again later."; + } + return fallback; +} diff --git a/mobile-app/sapot-mobile-app/features/shared/core/errors/index.ts b/mobile-app/sapot-mobile-app/features/shared/core/errors/index.ts index 9042aeb5..58fa5dd3 100644 --- a/mobile-app/sapot-mobile-app/features/shared/core/errors/index.ts +++ b/mobile-app/sapot-mobile-app/features/shared/core/errors/index.ts @@ -4,3 +4,10 @@ export { toAppError } from "./to-app-error"; export { KeyInitError, toKeyInitError } from "./key-init-error"; export type { KeyInitErrorCode } from "./key-init-error"; export { captureAppError } from "./sentry-capture"; +export { + GsmGatewayError, + getGsmErrorMessage, + getGsmFailure, + toGsmGatewayError, +} from "./gsm-error"; +export type { GsmFailure } from "./gsm-error"; diff --git a/mobile-app/sapot-mobile-app/features/shared/hooks/index.ts b/mobile-app/sapot-mobile-app/features/shared/hooks/index.ts index 2392fadc..333041d6 100644 --- a/mobile-app/sapot-mobile-app/features/shared/hooks/index.ts +++ b/mobile-app/sapot-mobile-app/features/shared/hooks/index.ts @@ -8,6 +8,7 @@ export * from "./use-connection-service"; export * from "./use-dialog-visibility"; export * from "./use-discovery-service"; export * from "./use-foreground-sync"; +export * from "./use-gsm-service"; export * from "./use-health-poll"; export * from "./use-loading-overlay"; export * from "./use-main-container"; @@ -24,4 +25,3 @@ export * from "./use-user-profile"; export * from "./use-user-search"; export * from "./use-user-store"; export * from "./use-zeroconf-published"; - diff --git a/mobile-app/sapot-mobile-app/features/shared/hooks/use-gsm-health.ts b/mobile-app/sapot-mobile-app/features/shared/hooks/use-gsm-health.ts index d310ded3..0534fb85 100644 --- a/mobile-app/sapot-mobile-app/features/shared/hooks/use-gsm-health.ts +++ b/mobile-app/sapot-mobile-app/features/shared/hooks/use-gsm-health.ts @@ -1,11 +1,12 @@ -import { getGsmHealth } from "@/features/shared/core/api/gsm.api"; import { hookLog } from "@/features/shared/core/utils/logger"; import { useEffect, useState } from "react"; +import { useGsmService } from "./use-gsm-service"; hookLog.debug("[use-gsm-health] module loaded"); const GSM_POLL_INTERVAL_MS = 30_000; export function useGsmHealth(): { gsmReady: boolean; loading: boolean } { + const gsmService = useGsmService(); const [gsmReady, setGsmReady] = useState(false); const [loading, setLoading] = useState(true); @@ -14,7 +15,7 @@ export function useGsmHealth(): { gsmReady: boolean; loading: boolean } { const check = (isInitial: boolean) => { if (isInitial) setLoading(true); - getGsmHealth() + gsmService.getHealth() .then((res) => { if (!cancelled) setGsmReady(res.gsm_ready === true); }) .catch(() => { if (!cancelled) setGsmReady(false); }) .finally(() => { if (isInitial && !cancelled) setLoading(false); }); @@ -27,7 +28,7 @@ export function useGsmHealth(): { gsmReady: boolean; loading: boolean } { cancelled = true; clearInterval(id); }; - }, []); + }, [gsmService]); return { gsmReady, loading }; } diff --git a/mobile-app/sapot-mobile-app/features/shared/hooks/use-gsm-service.ts b/mobile-app/sapot-mobile-app/features/shared/hooks/use-gsm-service.ts new file mode 100644 index 00000000..d5ece30b --- /dev/null +++ b/mobile-app/sapot-mobile-app/features/shared/hooks/use-gsm-service.ts @@ -0,0 +1,5 @@ +import { useMainContainer } from "./use-main-container"; + +export function useGsmService() { + return useMainContainer().gsmService; +} diff --git a/mobile-app/sapot-mobile-app/features/shared/main-container.ts b/mobile-app/sapot-mobile-app/features/shared/main-container.ts index a0b1fc4e..aee7c726 100644 --- a/mobile-app/sapot-mobile-app/features/shared/main-container.ts +++ b/mobile-app/sapot-mobile-app/features/shared/main-container.ts @@ -14,6 +14,7 @@ import { CleanUpService, ConnectionService, DiscoveryService, + GsmService, NotificationService, SignalingService, WebrtcSessionManager, @@ -79,6 +80,7 @@ export class MainContainer { readonly zeroconfAdapter: ZeroconfAdapter; readonly networkConfig: NetworkConfig; readonly discoveryService: DiscoveryService; + readonly gsmService: GsmService; readonly tcpServerAdapter: TcpServerAdapter; readonly webrtcSessionManager: WebrtcSessionManager; readonly signalingService: SignalingService; @@ -126,6 +128,7 @@ export class MainContainer { this.appModeStore = appModeStore; this.networkConfig = new NetworkConfig(); + this.gsmService = new GsmService(); this.localEncryptionService = new LocalEncryptionService({ getPassword: () => _pendingRawPassword, From 9fa6244e049db67193c74f7e8f38a9351ebebf43 Mon Sep 17 00:00:00 2001 From: Adamskiee Date: Fri, 14 Aug 2026 14:13:45 +0800 Subject: [PATCH 14/15] docs(docs-gsm): document resilient authenticated delivery --- SECURITY.md | 2 ++ docs/api/gsm-sms.md | 18 ++++++++++- docs/architecture/component-map.md | 2 +- docs/architecture/data-flow.md | 2 +- docs/architecture/threat-model.md | 2 +- docs/deployment/environment-config.md | 2 +- docs/deployment/gsm-module.md | 8 ++--- docs/deployment/server.md | 2 +- docs/features/sms-gateway/design.md | 30 +++++++++++++----- docs/features/sms-gateway/requirements.md | 17 ++++++++-- docs/features/sms-gateway/testing.md | 38 ++++++++++++++++++----- docs/getting-started/gsm-module-setup.md | 2 +- 12 files changed, 95 insertions(+), 30 deletions(-) diff --git a/SECURITY.md b/SECURITY.md index fcf96173..27b6aeaf 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -13,6 +13,7 @@ This is the canonical source of truth for SAPOT's known security-relevant config | Hardcoded JWT secret fallback | `server/app/db_operations/token.py` (`SECRET_KEY`) | The default value has been removed; `JWT_SECRET_KEY` is now required, and the app raises `RuntimeError` at import time if unset. **Rotate to a newly generated secret** (`openssl rand -hex 32`) — the old hardcoded value must be considered compromised since it was committed to source. | | CORS wildcard + credentials | `server/app/main.py` | `allow_origins=["*"]` replaced with an explicit allowlist read from `CORS_ALLOWED_ORIGINS` (comma-separated). The app raises `RuntimeError` at import time if unset. | | Testing router in production | `server/app/main.py`, `server/app/api/testing.py` | The router is imported and mounted only in `development` or `staging`. A router-wide dependency also returns 404 outside those environments if the router is mis-mounted. Every state-changing route requires the `X-QA-Token` shared secret. A production-process regression test exercises every testing path. | +| Unauthenticated direct GSM sends | `GSM-module/GSM-fastapi/api.py`, `server/app/api/gsm.py` | `GSM_SECRET` is required by both services. The main server sends it as `X-GSM-Secret`, and the gateway validates it before logging or queueing `POST /sms/send`. | ## Required environment variables (new) @@ -29,6 +30,7 @@ Set this in `/etc/sapot/gsm.env` before starting the GSM service: ```dotenv DB_PATH=mysql+pymysql://:@127.0.0.1:3306/sapot_db +GSM_SECRET= ``` QA-enabled environments also require `QA_API_TOKEN`. Generate a strong random value and send it as `X-QA-Token` for state-changing `/testing/*` requests. Production does not load this secret. diff --git a/docs/api/gsm-sms.md b/docs/api/gsm-sms.md index ed0553d5..3d7d942d 100644 --- a/docs/api/gsm-sms.md +++ b/docs/api/gsm-sms.md @@ -41,6 +41,22 @@ Webhook endpoint the GSM hardware gateway calls when it receives an SMS. Protect --- -The GSM module's own standalone hardware-facing API (separate service) is documented in [`docs/deployment/gsm-module.md`](../deployment/gsm-module.md). +The GSM module's own standalone hardware-facing API is documented in [`docs/deployment/gsm-module.md`](../deployment/gsm-module.md). Its `POST /sms/send` route requires the same `X-GSM-Secret` shared secret that protects the main server's inbound webhook. + +## Gateway failure contract + +The main server preserves synchronous gateway failures for `/gsm/sms/send`, `/gsm/request`, `/gsm/resend`, and `/gsm/contact-unknown-user`. Queue saturation returns HTTP 503: + +```json +{ + "detail": { + "message": "Outbound SMS queue is full", + "reason": "QUEUE_FULL", + "msg_id": "" + } +} +``` + +An unreachable gateway also returns HTTP 503 with `reason: "GATEWAY_UNAVAILABLE"`. Clients should keep rejected messages available for manual retry and should not report verification or onboarding SMS as sent. See [gsm-sms.yaml](openapi/gsm-sms.yaml) for exact field-level request/response schemas, or the live server's `/docs` / `/openapi.json`. diff --git a/docs/architecture/component-map.md b/docs/architecture/component-map.md index 9ce25e4a..b65500e5 100644 --- a/docs/architecture/component-map.md +++ b/docs/architecture/component-map.md @@ -86,7 +86,7 @@ For the security trust boundaries overlaid on this same topology (which zones ar | `/static/` | Filesystem | Served directly by Nginx; 30-day cache | | `/tiles/` | `http://127.0.0.1:8080` | Tileserver styles; prefix stripped by trailing `/` on `proxy_pass` | | `/data/`, `/fonts/`, `/sprites/` | `http://127.0.0.1:8080` | TileServer GL assets referenced by its absolute style URLs | -| `/` (all other) | `http://127.0.0.1:8000` | Standard proxy; 135 s read timeout | +| `/` (all other) | `http://127.0.0.1:8000` | Standard proxy; 155 s read timeout | HTTP (port 80) redirects to HTTPS with 301. diff --git a/docs/architecture/data-flow.md b/docs/architecture/data-flow.md index 867c6289..d58884e8 100644 --- a/docs/architecture/data-flow.md +++ b/docs/architecture/data-flow.md @@ -126,7 +126,7 @@ sequenceDiagram S->>S: resolve sender/target user, create/append SMS conversation ``` -The main server authenticates user-facing outbound requests with a JSON Web Token (JWT). The direct GSM send endpoint is restricted to the trusted host or Compose network and does not require `X-GSM-Secret`. The shared secret authenticates only the GSM module's inbound callback to `/gsm/inbound`; see [environment-config.md](../deployment/environment-config.md). +The main server authenticates user-facing outbound requests with a JSON Web Token (JWT). Calls across the server and GSM service boundary use `X-GSM-Secret` in both directions: the main server sends it to `/sms/send`, and the GSM service sends it to `/gsm/inbound`. Network restriction to the host or trusted Compose network remains an additional boundary; see [environment-config.md](../deployment/environment-config.md). --- diff --git a/docs/architecture/threat-model.md b/docs/architecture/threat-model.md index ca3e13be..6c4afd22 100644 --- a/docs/architecture/threat-model.md +++ b/docs/architecture/threat-model.md @@ -66,7 +66,7 @@ flowchart TB | mDNS/Zeroconf discovery | Broadcasts peer presence and connection info on the LAN; unauthenticated by design (mDNS has no auth mechanism) | | Captive portal | The first thing an unauthenticated device interacts with; controls initial network admission | | Admin frontend | Higher-privilege surface — user management, announcements, network config | -| GSM module webhook (`/gsm/inbound`) | Authenticated via shared secret (`GSM_SECRET`/`X-GSM-Secret`); reachable from the server, and from the GSM module's own network segment | +| GSM service HTTP boundary (`/sms/send`, `/gsm/inbound`) | Authenticated in both directions via shared secret (`GSM_SECRET`/`X-GSM-Secret`); reachable from the server and the GSM module's network segment | | MariaDB, Redis | Server-internal; in scope only via server compromise (not directly LAN-reachable in the documented deployment) | ## Attack surfaces explicitly out of scope diff --git a/docs/deployment/environment-config.md b/docs/deployment/environment-config.md index 17f9f339..c1c826a6 100644 --- a/docs/deployment/environment-config.md +++ b/docs/deployment/environment-config.md @@ -59,7 +59,7 @@ GSM_SECRET= | `PORT` | `8000` (code default in `config.py`), but **not actually read** — `GSM-fastapi/main.py` hardcodes `uvicorn.run(..., port=8001, ...)` regardless of this variable. The service always listens on `8001` in practice, which is what avoids colliding with the main SAPOT server on `127.0.0.1:8000` — not the `PORT` variable. | Not a real configuration knob today — see `GSM-module/CLAUDE.md`'s "Common Pitfalls" | | `LOG_LEVEL` | `INFO` | Python logging level (`config.py`) | | `SAPOT_API_URL` | `http://localhost:8000` | Base URL the GSM module uses to call back into the SAPOT server (`database.py`) — must match wherever the server actually listens | -| `GSM_SECRET` | `""` (empty) | Shared secret sent by the GSM module when it calls the server's `/gsm/inbound` route. **Must match the server's `GSM_SECRET`** (see above) | +| `GSM_SECRET` | None; startup raises `RuntimeError` when unset | Shared secret validated for server calls to `/sms/send` and sent by the GSM module on `/gsm/inbound` callbacks. **Must match the server's `GSM_SECRET`** (see above) | | `SMS_BOT_USER_ID` | unset | User ID the GSM module attributes inbound SMS-originated messages to, when the sender can't be resolved to a registered user (`database.py`) | | `SMS_SEND_QUEUE_MAXSIZE` | `10` | Maximum waiting outbound requests. Integers from `1` through `20` are accepted; other values fail startup. The upper bound leaves capacity in FastAPI's default 40-thread worker pool so overload requests can reach the non-blocking admission check. | diff --git a/docs/deployment/gsm-module.md b/docs/deployment/gsm-module.md index e4937618..49128ca5 100644 --- a/docs/deployment/gsm-module.md +++ b/docs/deployment/gsm-module.md @@ -16,8 +16,8 @@ The GSM module (`GSM-module/GSM-fastapi/`) is a FastAPI application that bridges ```bash cd GSM-module/GSM-fastapi/ -python3 -m venv venv -source venv/bin/activate +nix develop --command python -m venv venv +nix develop pip install -r requirements.txt cp .env.example .env # Edit .env and set DB_PATH, GSM_SECRET, SAPOT_API_URL, and the serial device. @@ -34,7 +34,7 @@ bash run-api.sh ### Docker (dev/test alternative) -The root `docker-compose.yml` (see [docker-setup.md](../getting-started/docker-setup.md)) includes a `gsm-fastapi` service alongside the rest of the stack. The base file does not pass through `/dev/ttyACM0`, which lets development stacks start without GSM hardware. Add `docker-compose.gsm-hardware.yml` when the modem is attached. The service listens on all interfaces inside its container, but Compose publishes port 8001 only on host loopback because the direct API is unauthenticated. +The root `docker-compose.yml` (see [docker-setup.md](../getting-started/docker-setup.md)) includes a `gsm-fastapi` service alongside the rest of the stack. The base file does not pass through `/dev/ttyACM0`, which lets development stacks start without GSM hardware. Add `docker-compose.gsm-hardware.yml` when the modem is attached. The service listens on all interfaces inside its container, validates `X-GSM-Secret` for direct sends, and publishes port 8001 only on host loopback as an additional network boundary. --- @@ -49,7 +49,7 @@ The root `docker-compose.yml` (see [docker-setup.md](../getting-started/docker-s | `HOST` | `127.0.0.1` | FastAPI bind host | | `PORT` | `8000` in `config.py`, but not used for binding | `main.py` always binds port `8001`; do not rely on this setting | | `SAPOT_API_URL` | `http://localhost:8000` | Base URL for authenticated inbound callbacks to the main server | -| `GSM_SECRET` | Empty string | Must match the main server value in production | +| `GSM_SECRET` | None | Required at startup; must match the main server value | > **Security note:** Set `DB_PATH` and `GSM_SECRET` explicitly before startup. Never deploy the placeholder credentials from `.env.example`. See [secrets-management.md](secrets-management.md). diff --git a/docs/deployment/server.md b/docs/deployment/server.md index 17235a40..85940358 100644 --- a/docs/deployment/server.md +++ b/docs/deployment/server.md @@ -140,7 +140,7 @@ If you are pointing the server at a database created *before* Alembic was adopte |---|---| | `/ws/` | WebSocket proxy; no read timeout (86400 s) | | `/static/` | Filesystem; 30-day cache | -| `/` | Standard proxy; 135 s read timeout | +| `/` | Standard proxy; 155 s read timeout | Port 80 redirects to HTTPS (301). TLS 1.2/1.3, cipher `HIGH:!aNULL:!MD5`. diff --git a/docs/features/sms-gateway/design.md b/docs/features/sms-gateway/design.md index 811edf7d..3db8906e 100644 --- a/docs/features/sms-gateway/design.md +++ b/docs/features/sms-gateway/design.md @@ -22,7 +22,7 @@ sequenceDiagram participant Arduino Client->>Main: POST /gsm/sms/send - Main->>GSM: POST /sms/send {number, body} + Main->>GSM: POST /sms/send with X-GSM-Secret GSM->>GSM: log pending and admit to bounded queue GSM->>Arduino: SEND_SMS|number|body Arduino-->>GSM: SMS_SENT|number or SMS_FAILED|number|reason @@ -33,7 +33,7 @@ sequenceDiagram GSM->>Main: POST /gsm/inbound with X-GSM-Secret ``` -The main server authenticates the user-facing `/gsm/sms/send` route with a JSON Web Token (JWT). The GSM service normally listens on `127.0.0.1:8001` and translates trusted local HTTP calls into serial commands. +The main server authenticates the user-facing `/gsm/sms/send` route with a JSON Web Token (JWT). It authenticates its direct gateway call with the same shared `GSM_SECRET` used for callbacks. The GSM service validates `X-GSM-Secret` before logging or queueing a send, which prevents another container on the internal network from occupying the serial modem. ## How does outbound admission work? @@ -53,15 +53,20 @@ Admission uses `put_nowait()` while the lifecycle lock is held. A full queue rai The upper limit leaves worker threads available for overload responses and other synchronous FastAPI routes. `GET /health` is asynchronous, so liveness remains responsive while admitted sends wait for modem results. +Each request receives a pre-write deadline when it is admitted. Queue wait and modem-readiness wait share that deadline. If the caller reaches it first, it marks the request complete while synchronized with the active-to-in-flight transition, so the sender cannot write that request later. + +Starting the serial write moves the request to a separate confirmation deadline. The caller follows that transition even when the serial write crosses the pre-write deadline, so it cannot report a retryable queue timeout after bytes have reached the Arduino. + ## How is one SMS sent? The sender owns one active request at a time: 1. Dequeue and register the request as active. -2. Wait for modem readiness up to the request timeout. +2. Wait for modem readiness within the remaining admission deadline. 3. Atomically transition the request to in-flight while writing `SEND_SMS||`. -4. Wait for `SMS_SENT` or `SMS_FAILED` from the reader thread. -5. Complete the matching caller and let the API update `sms_log`. +4. Start a fresh confirmation deadline after the serial write completes. +5. Wait for `SMS_SENT` or `SMS_FAILED` within the confirmation deadline. +6. Complete the matching caller and let the API update `sms_log`. The serial connection has a five-second write timeout. Reader events cannot complete a request before its serial write begins. Queue depth excludes the active request. @@ -106,6 +111,12 @@ The GSM service uses the database configured by required `DB_PATH`. The committed `sapot.db` file is stale and is not used by the deployed service. +## How does startup recover interrupted work? + +After database initialization and before constructing `SerialWorker`, the lifespan calls `fail_orphaned_pending_messages()`. One database update changes every `pending` `sms_log` row to `failed` with `SERVICE_CRASHED`. + +The gateway does not re-queue these rows. A crash can happen after the modem transmits an SMS but before the process records its confirmation, so replay could deliver duplicate emergency messages. + ## How are failures reported? | Failure | Result | @@ -115,14 +126,17 @@ The committed `sapot.db` file is stale and is not used by the deployed service. | Serial port or modem unavailable before admission | HTTP 503 | | Serial write error | HTTP 502 with a reason beginning `WRITE_ERROR:` | | Modem reports failure or confirmation times out | HTTP 502 with the modem or timeout reason | +| Caller deadline expires while waiting in the queue | Request fails and is never written later | | Main server cannot reach the GSM service | Main server health route returns HTTP 503 | -The main server currently returns the GSM response body without preserving the upstream status from `POST /sms/send`. Callers using the main server route cannot rely on the gateway's 503 status until that proxy behavior is corrected. +The main server preserves the gateway status and error detail for synchronous SMS operations. Its 135-second read timeout covers the gateway's 125-second worst case plus HTTP overhead. Nginx allows 155 seconds so the one-second pool, five-second connect, five-second write, and 135-second read phase limits all fit inside the outer proxy limit. The main server permits 22 GSM connections, enough for 21 admitted gateway requests plus one request that observes `QUEUE_FULL`. Further requests fail pool admission within one second and never reach the gateway later. + +The mobile app maps `QUEUE_FULL` to a busy message, keeps a rejected chat message as `not_sent`, and offers its existing manual resend action. Phone verification, resend, and first-contact screens remain in place and show the gateway failure instead of reporting success. ## Security and deployment assumptions - Set `DB_PATH` and `GSM_SECRET` in restricted environment files. Bare-metal systemd deployments use `/etc/sapot/gsm.env`. -- The main server checks `X-GSM-Secret` on inbound callbacks. -- The direct GSM service does not authenticate `/sms/send`; keep port 8001 restricted to the host or trusted Compose network. +- The main server and GSM service check `X-GSM-Secret` on both directions of their HTTP integration. +- Keep port 8001 restricted to the host or trusted Compose network as an additional boundary. - SMS content is plaintext on the carrier network and should not be treated as end-to-end encrypted. - The design supports one serial modem. Multi-modem failover and bulk SMS are out of scope. diff --git a/docs/features/sms-gateway/requirements.md b/docs/features/sms-gateway/requirements.md index 8046eb47..61bdd05f 100644 --- a/docs/features/sms-gateway/requirements.md +++ b/docs/features/sms-gateway/requirements.md @@ -28,6 +28,10 @@ These requirements describe the deployed `GSM-module/GSM-fastapi/` service and i } ``` +The request must include `X-GSM-Secret` matching the required `GSM_SECRET` +configuration. Missing or invalid credentials return HTTP 401 before the +gateway creates a log row or admits work to the serial queue. + The number must use E.164 format. The submitted body must not exceed 160 characters and must remain nonempty after trimming. A successful modem confirmation returns HTTP 200: @@ -49,6 +53,9 @@ A modem failure, write failure, or confirmation timeout returns HTTP 502 and rec - Waiting requests retain first-in, first-out order. - Admission must not block when the queue is full. - Work beyond capacity must never reach the serial port. +- The pre-write timeout starts at admission, not when the request reaches the front of the queue. +- A waiting request whose caller-visible deadline expires must never be written later. +- Once a serial write starts, the caller must wait for modem confirmation or the post-write confirmation timeout instead of reporting the pre-write timeout. - Saturated requests must be logged as failed and return HTTP 503 with `reason: "QUEUE_FULL"`. ```json @@ -95,23 +102,28 @@ Only one request may await a modem confirmation. A confirmation received before - The serial reader must enqueue `SMS_RECEIVED` events for application processing. - `handle_incoming_sms()` must apply the registered-user, banned-user, verified-phone, session, and target rules. +- Sender eligibility failures must set the inbound `sms_log` row to `rejected` with `NO_ACCOUNT`, `BANNED_SENDER`, or `UNVERIFIED_SENDER` as the failure reason. - The GSM service must call the main server's `POST /gsm/inbound` route with `X-GSM-Secret` when forwarding into the app. - Failed callbacks are logged. Automatic callback retry is not required. ### FR-SG-07: Configuration and storage - `DB_PATH` is required. Startup must raise `RuntimeError` when it is missing. +- `GSM_SECRET` is required. Startup must raise `RuntimeError` when it is missing. - Invalid `SMS_SEND_QUEUE_MAXSIZE` values must fail startup. - `sms_log` stores inbound and outbound audit records. - `sms_session` stores per-phone relay state. +- Before starting `SerialWorker`, startup must change every orphaned `pending` log row to `failed` with `SERVICE_CRASHED`. +- Startup reconciliation must not re-queue orphaned messages because the modem may have transmitted them before the prior process stopped. - The committed `sapot.db` file must not be used as the deployment datastore. ### FR-SG-08: Main server integration - The user-facing `/gsm/sms/send` route remains on the main server and requires its normal JWT authentication. -- The main server calls the direct gateway at `http://localhost:8001/sms/send`. +- The main server calls the direct gateway at `http://localhost:8001/sms/send` with `X-GSM-Secret`. - The direct gateway is a trusted local service and must not be exposed to untrusted networks. -- Preserving the gateway's HTTP status through the main server proxy is a known limitation, not part of this change. +- The main server must preserve gateway HTTP 502 and 503 failures for user-facing send, verification, resend, and first-contact requests. +- The mobile app must retain rejected chat messages as `not_sent` and distinguish queue saturation from a generic delivery failure. ## Non-functional requirements @@ -130,4 +142,3 @@ Only one request may await a modem confirmation. A confirmation received before - Multimedia Messaging Service (MMS) - Carrier delivery or read receipts - Automatic retry of failed main-server callbacks -- Preserving direct-gateway HTTP status through the current main-server proxy diff --git a/docs/features/sms-gateway/testing.md b/docs/features/sms-gateway/testing.md index ae061a17..256d0109 100644 --- a/docs/features/sms-gateway/testing.md +++ b/docs/features/sms-gateway/testing.md @@ -6,7 +6,9 @@ The GSM FastAPI tests verify queue admission, lifecycle races, API responses, an ## Prerequisites -Use the component's pinned Nix environment and installed virtual environment: +Run each touched component in its pinned environment. + +GSM gateway: ```bash cd GSM-module/GSM-fastapi @@ -14,23 +16,41 @@ nix develop pytest ``` -When Nix is unavailable, the exact pinned `requirements.txt` can be exercised in an isolated environment: +Main server proxy contract: + +```bash +cd server +nix develop +cd app +pytest tests/test_gsm_health.py tests/test_gsm_proxy.py +``` + +Mobile full component gate, including GSM error handling and dependency injection wiring: ```bash -uv run --isolated --with-requirements requirements.txt pytest +cd mobile-app +nix develop +cd sapot-mobile-app +pnpm run testAll ``` -`tests/conftest.py` supplies a test `DB_PATH` before importing application settings. Tests that reach API handlers replace database operations with fakes, so they do not create or mutate production records. +`tests/conftest.py` supplies test `DB_PATH` and `GSM_SECRET` values before importing application settings. Tests that reach API handlers replace database operations with fakes, so they do not create or mutate production records. ## What is covered? | File | Responsibility | |---|---| | `tests/test_config.py` | Default queue capacity, valid range, and startup rejection | -| `tests/test_serial_worker.py` | Queue capacity, lifecycle cutoff, exact-once completion, pre-write races, and serial write timeout | +| `tests/test_serial_worker.py` | Queue capacity, admission deadlines, lifecycle cutoff, exact-once completion, pre-write races, and serial write timeout | | `tests/test_api_queue.py` | HTTP 503 contracts, message-log updates, diagnostics, worker-pool headroom, and health responsiveness | - -The suite currently contains 24 focused tests after the outbound-queue change. +| `tests/test_database_reconciliation.py` | Idempotent startup recovery of orphaned pending log rows | +| `tests/test_incoming_sms.py` | Sender rejection reason codes and inbound log status updates | +| `tests/test_lifespan.py` | Reconciliation ordering before serial worker startup | +| `server/app/tests/test_gsm_proxy.py` | Main-server shared-secret header, status preservation, and timeout headroom for chat, verification, resend, and first-contact requests | +| `mobile-app/sapot-mobile-app/features/shared/core/errors/__tests__/gsm-error.test.ts` | Typed `QUEUE_FULL` parsing and user-visible error messages | +| `mobile-app/sapot-mobile-app/features/chat/components/__tests__/message-list.test.tsx` | Manual resend rejection and `not_sent` restoration | +| `mobile-app/sapot-mobile-app/features/auth/auth-container.test.ts` | Phone-verification service construction | +| `mobile-app/sapot-mobile-app/features/shared/__tests__/main-container-initialize.test.ts` | GSM service construction within the runtime container | ## How is serial I/O isolated? @@ -47,6 +67,8 @@ No test may rely on `/dev/ttyACM0`, a SIM card, or a carrier network. A test tha | Shutdown begins with waiting work | Waiting requests complete with `SERVICE_STOPPING` | | Shutdown begins with active work | Active request completes with `SERVICE_STOPPING` | | Network loss completes active work before writing | Recovery does not write the failed request | +| Caller deadline expires while a request is queued | The request returns `CLIENT_TIMEOUT` and is never written | +| Serial write crosses the admission deadline | The caller waits for modem confirmation instead of returning the pre-write timeout | | Stale confirmation arrives before a new write | New request remains pending and is written normally | | Two completions race for one request | First completion wins | | Serial connection opens | PySerial receives the configured five-second write timeout | @@ -78,6 +100,7 @@ Run this only on a host with the configured Arduino and SIM: ```bash curl -X POST http://127.0.0.1:8001/sms/send \ -H 'Content-Type: application/json' \ + -H 'X-GSM-Secret: ' \ -d '{"number":"+639171234567","body":"SAPOT GSM smoke test"}' ``` @@ -87,5 +110,4 @@ Run this only on a host with the configured Arduino and SIM: - Automated tests do not prove USB permissions, modem readiness, SIM balance, signal quality, or carrier delivery. - API tests mock message-log persistence; they do not validate the MariaDB schema. -- The suite does not test whether the main server preserves the direct gateway's HTTP status. It currently does not. - Real-hardware testing must use a controlled phone number and must not run in shared CI. diff --git a/docs/getting-started/gsm-module-setup.md b/docs/getting-started/gsm-module-setup.md index a6575cd7..5141c188 100644 --- a/docs/getting-started/gsm-module-setup.md +++ b/docs/getting-started/gsm-module-setup.md @@ -42,7 +42,7 @@ cp .env.example .env | Variable | Default | Notes | |---|---|---| | `DB_PATH` | none, **required** | SQLModel URL for the server's MariaDB, e.g. `mysql+pymysql://sapot:sapot@127.0.0.1:3306/sapot_dev`. `config.py` raises `RuntimeError` at import if unset. | -| `GSM_SECRET` | `""` | Must match the server's `GSM_SECRET`. The gateway sends it as `X-GSM-Secret` when calling `/gsm/inbound`; the server rejects a missing or mismatched value. | +| `GSM_SECRET` | None | Required at startup and must match the server's `GSM_SECRET`. Both services send it as `X-GSM-Secret` when calling the other service. | | `SAPOT_API_URL` | `http://localhost:8000` | Base URL of the SAPOT server this gateway forwards inbound SMS to (`POST /gsm/inbound`). The Docker service overrides it to `https://nginx`. | | `SERIAL_PORT` | `/dev/ttyACM0` | Serial device the Arduino is on. `COM3`-style on Windows. | | `SERIAL_BAUD` | `9600` | Must match `PC_BAUD` in the Arduino sketch. | From b7c818438d877b135422af18614b7ce3308193b1 Mon Sep 17 00:00:00 2001 From: Adamskiee Date: Fri, 14 Aug 2026 17:07:25 +0800 Subject: [PATCH 15/15] feat(server-testing): seed verified phone fixture --- docs/qa/scenario-tooling.md | 1 + server/app/api/testing.py | 1 + server/app/db_operations/qa_scenarios.py | 14 ++++++++++++++ server/app/tests/test_qa_scenarios.py | 11 +++++++++++ server/app/tests/test_testing_endpoints.py | 10 ++++++++++ 5 files changed, 37 insertions(+) diff --git a/docs/qa/scenario-tooling.md b/docs/qa/scenario-tooling.md index b7065624..058e4c22 100644 --- a/docs/qa/scenario-tooling.md +++ b/docs/qa/scenario-tooling.md @@ -24,6 +24,7 @@ Router: [`server/app/api/testing.py`](../../server/app/api/testing.py). Scenario | `large` | `qa_large` + many peers/messages/GPS points, for list perf and sync-cursor testing | | `banned` | `qa_banned` with an active `BannedUser` row | | `locked-out` | `qa_locked` with a `LoginAttempt` row at the lockout threshold | +| `verified-phone` | `qa_phone_verified` with a `PhoneVerified` row, for phone-verification-gated flows | | `announcements` | Active + expired announcements across all priorities and audiences | | `gps-track` | `qa_gps` with a 60-point location history along a route | | `calls` | `qa_calls_a` / `qa_calls_b` with completed/missed/rejected call rows | diff --git a/server/app/api/testing.py b/server/app/api/testing.py index 84b9d968..c9813419 100644 --- a/server/app/api/testing.py +++ b/server/app/api/testing.py @@ -39,6 +39,7 @@ "qa_large", "qa_banned", "qa_locked", + "qa_phone_verified", "qa_gps", "qa_map_user", "qa_map_user_2", diff --git a/server/app/db_operations/qa_scenarios.py b/server/app/db_operations/qa_scenarios.py index 09f1efdd..5a81a25d 100644 --- a/server/app/db_operations/qa_scenarios.py +++ b/server/app/db_operations/qa_scenarios.py @@ -28,6 +28,7 @@ from app.models.location import UserLocation from app.models.login_attempt import LoginAttempt from app.models.message import Message, MessageType +from app.models.phone_verification import PhoneVerified from app.models.rescuer import Rescuer from app.models.users import User @@ -423,6 +424,15 @@ def build_locked_out(session: Session) -> dict: return {"user": user.username, "locked_until": locked_until.isoformat()} +def build_verified_phone(session: Session) -> dict: + user = get_or_create_user(session, "qa_phone_verified", phone_number="+639300000751") + verified = session.exec(select(PhoneVerified).where(PhoneVerified.user_id == user.id)).first() + if not verified: + session.add(PhoneVerified(user_id=user.id)) + session.commit() + return {"user": user.username, "phone_verified": True} + + _ANNOUNCEMENT_PRIORITIES = (PriorityType.low, PriorityType.normal, PriorityType.high) _ANNOUNCEMENT_AUDIENCES = (AudienceType.user, AudienceType.rescuer, AudienceType.admin) @@ -592,6 +602,10 @@ class Scenario(NamedTuple): "qa_locked with a LoginAttempt row at attempt_count=5, locked_until +6h.", build_locked_out, ), + "verified-phone": Scenario( + "qa_phone_verified with a verified Philippine phone number.", + build_verified_phone, + ), "announcements": Scenario( "Active + expired announcements across all 3 priorities x 3 audiences (18 rows).", build_announcements, diff --git a/server/app/tests/test_qa_scenarios.py b/server/app/tests/test_qa_scenarios.py index 3bbda6d1..ba4f4305 100644 --- a/server/app/tests/test_qa_scenarios.py +++ b/server/app/tests/test_qa_scenarios.py @@ -14,6 +14,7 @@ from app.models.location import UserLocation from app.models.login_attempt import LoginAttempt from app.models.message import Message +from app.models.phone_verification import PhoneVerified from app.models.rescuer import Rescuer from app.models.users import User @@ -79,6 +80,16 @@ def test_build_locked_out_sets_attempt_count_and_lock(session: Session): assert attempt.locked_until.replace(tzinfo=None) > datetime.now(timezone.utc).replace(tzinfo=None) +def test_build_verified_phone_creates_verified_user(session: Session): + result = qa_scenarios.build_verified_phone(session) + + user = session.exec(select(User).where(User.username == "qa_phone_verified")).first() + assert user is not None + assert user.phone_number == "+639300000751" + assert session.exec(select(PhoneVerified).where(PhoneVerified.user_id == user.id)).first() + assert result == {"user": "qa_phone_verified", "phone_verified": True} + + def test_build_announcements_covers_priority_and_audience_matrix(session: Session): qa_scenarios.build_announcements(session) diff --git a/server/app/tests/test_testing_endpoints.py b/server/app/tests/test_testing_endpoints.py index e2e93cf6..73ebe352 100644 --- a/server/app/tests/test_testing_endpoints.py +++ b/server/app/tests/test_testing_endpoints.py @@ -43,6 +43,16 @@ def test_seed_gps_roles_scenario_creates_multi_role_fixtures(client): assert body["result"]["rescuers"] == ["qa_map_rescuer", "qa_map_rescuer_2"] +def test_seed_verified_phone_scenario_creates_login_fixture(client): + response = client.post("/testing/seed/verified-phone", headers=QA_HEADERS) + assert response.status_code == 200 + assert response.json()["result"] == {"user": "qa_phone_verified", "phone_verified": True} + + login = client.post("/testing/login-as/qa_phone_verified", headers=QA_HEADERS) + assert login.status_code == 200 + assert login.json()["username"] == "qa_phone_verified" + + def test_login_as_qa_map_rescuer_mints_usable_tokens(client): client.post("/testing/seed/gps-roles", headers=QA_HEADERS)