Skip to content

Avoid incorrect SMS delivery-status updates when TP-MR repeats #294

Description

@Justinabox

Problem

SMSStore can attach a delivery report to the wrong outbound SMS when a modem reuses a TP-MR / +CMGS reference for the same recipient.

A delivery report currently contains only reference, recipient, and normalized status when it reaches SMSStore. SMSStore._latest_matching_message() then scans in reverse and selects the newest stored SMS whose (reference, recipient) matches. Since modem message references are finite and can repeat, a delayed receipt for an older outbound message can mark a newer same-recipient message as delivered or failed.

This creates incorrect delivery history for unattended notification/MFA integrations. It is preferable to leave a report uncorrelated than to mutate the status of a different logical SMS.

Reproduction (current main 6dd4211173857e6da55e6e2d0984c06fc56f9a7c)

PYTHONPATH=. uv run --no-project --with pytest --with pytest-asyncio --with pytest-aiohttp --with pyserial-asyncio --with aiosqlite python - <<'PY'
import asyncio
from callstack.sms.store import SMSStore
from callstack.sms.types import DeliveryReport, SMS

async def main():
    store = SMSStore()
    older = await store.save(SMS(
        recipient="synthetic-recipient", body="older", status="sent", reference=7
    ))
    newer = await store.save(SMS(
        recipient="synthetic-recipient", body="newer", status="sent", reference=7
    ))
    report = await store.save_delivery_report(DeliveryReport(
        reference=7, recipient="synthetic-recipient", status="delivered"
    ))
    print(report.message_id, [(m.id, m.status) for m in await store.list()])

asyncio.run(main())
PY

Observed output:

2 [(1, 'sent'), (2, 'delivered')]

The report is unconditionally attached to the newer message. This behavior is also codified in tests/test_sms_store.py::test_save_delivery_report_correlates_latest_matching_outbound_sms.

Root cause

  • callstack/sms/service.py::_parse_cmgr_status_report() discards the SCTS/discharge-time fields from the text-mode +CMGR delivery report.
  • callstack/sms/store.py::SMSStore._latest_matching_message() treats (reference, recipient) as unique and returns the last matching record.
  • callstack/sms/store.py::_save_delivery_report_locked() assigns that result to report.message_id and updates the chosen message's status.

Suggested fix direction

Make status correlation fail closed when the (reference, recipient) key is ambiguous. A small safe first slice could:

  1. Preserve parsed report timestamps where available and retain outbound submission timestamps.
  2. Correlate only a unique, temporally plausible candidate; otherwise persist the delivery report with message_id=None and do not update an outbound message status.
  3. Keep the existing unique-match path and the public delivery-report event behavior intact.

Do not use insertion order as a proxy for modem/carrier correlation.

Acceptance criteria

  • A unique matching outbound SMS is still linked and receives the delivery status.
  • Two unresolved outbound SMS records with the same recipient and reference do not cause a delayed receipt to update the newer record solely because it was inserted last.
  • Ambiguous reports remain durably listed for operator inspection, with message_id=None (or an explicit documented ambiguous state), and no false outbound status mutation.
  • If timestamp-based disambiguation is implemented, boundary tests cover late receipts and reject implausible matches rather than guessing.
  • In-memory and SQLite-backed stores preserve the chosen behavior across reopen.
  • Tests use synthetic values only and error/log paths remain PII-safe.

Affected files

  • callstack/sms/service.py
  • callstack/sms/store.py
  • callstack/sms/types.py (only if correlation metadata needs extension)
  • tests/test_delivery_reports.py
  • tests/test_sms_store.py

Verification gates

git diff --check
PYTHONPATH=. uv run --no-project --with pytest --with pytest-asyncio --with pytest-aiohttp --with pyserial-asyncio --with aiosqlite pytest tests/test_delivery_reports.py tests/test_sms_store.py -q
PYTHONPATH=. uv run --no-project --with pytest --with pytest-asyncio --with pytest-aiohttp --with pyserial-asyncio --with aiosqlite pytest tests/ -q

Non-goals

  • Changing modem message-reference allocation.
  • Aggregating multipart segment receipts.
  • Retrying delivery reports or changing SIM-slot cleanup semantics.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions