diff --git a/scripts/inbound_store.py b/scripts/inbound_store.py index 89d1d49..dba7158 100644 --- a/scripts/inbound_store.py +++ b/scripts/inbound_store.py @@ -14,10 +14,12 @@ 2. **A delivery ledger.** Each message carries a ``kb:delivered`` flag. This is **not** "read" — it records only whether the message has yet been *handed to - triage*. The flag is owned solely by the gateway: it is flipped to ``true`` - by exactly one operation, :func:`undelivered`, which returns the held - messages **and marks them delivered as a side effect**. Nothing else — no - SPARQL query, no ad-hoc read — ever touches it, so browsing history never + triage*. The flag is owned solely by the gateway and flipped ``false → true`` + by exactly two operations, both here: :func:`undelivered`, which returns the + held messages **and marks them delivered as a side effect** (the daily drain), + and :func:`mark_delivered`, which flips one already-written message the gateway + persisted up front (the persist-before-forward path — see below). Nothing else + — no SPARQL query, no ad-hoc read — ever touches it, so browsing history never silently "consumes" a message. The daily triage skill drains the backlog by calling the gateway's ``/undelivered`` endpoint (which calls this), so a message that arrived while its sender was not yet whitelisted is caught the @@ -25,11 +27,14 @@ The delivered flag lets a gateway persist a message it deliberately did **not** forward — a blacklisted or no-action-class sender is written straight to -``delivered: true`` (already accounted for, never drained), while an unknown or -whitelisted sender that *was* forwarded live is also written ``delivered: true`` -(triage already has it). Only a message that was persisted but **not** handed to -triage — e.g. a gateway that stored first and then found the model unreachable — -stays ``delivered: false`` and is picked up by the daily drain. +``delivered: true`` (already accounted for, never drained). A message that *is* +forwarded takes the never-drop path: the gateway writes it ``delivered: false`` +the instant it arrives (before the gate, before the forward — so a crash or a +throwing forward cannot lose it), then calls :func:`mark_delivered` once triage +actually has it. Any message that was persisted but **not** handed to triage — +a failed forward, a gateway that died mid-dispatch — stays ``delivered: false`` +and is picked up by the daily drain (at-least-once: a rare duplicate surface +beats a silent loss). Stdlib only (``hashlib``/``secrets``/``datetime``): this module is copied into each gateway image alongside ``triage_policy.py`` and ``reply_tokens.py``, and @@ -57,11 +62,22 @@ P_TEXT = KB + "text" P_MESSAGE_ID = KB + "messageId" P_DELIVERED = KB + "delivered" +# Optional reference to a retained raw-media file (e.g. a voice note's audio), +# recorded when a message is persisted *before* transcription so a failed or +# crashed STT run leaves a re-transcribable artifact instead of a silent drop. +# Cleared once the message is accounted for (transcribed and forwarded). +P_MEDIA = KB + "media" # Subdirectory (under the gateway's store dir) that holds the per-message files. # The gateway owns this folder read-write; the life store mounts it read-only. MESSAGES_SUBDIR = "messages" +# Subdirectory holding raw media (voice-note audio) retained for a message that +# was persisted before transcription. It lives beside the messages so it shares +# the gateway's durable data volume; the reference is recorded via P_MEDIA and +# the file is unlinked once the message is transcribed and accounted for. +MEDIA_SUBDIR = "media" + _SLUG_RE = re.compile(r"[^a-z0-9]+") @@ -74,6 +90,10 @@ def messages_dir(store_dir: str | Path) -> Path: return Path(store_dir) / MESSAGES_SUBDIR +def media_dir(store_dir: str | Path) -> Path: + return Path(store_dir) / MEDIA_SUBDIR + + # -- N-Triples serialization -------------------------------------------------- # A tiny, self-contained N-Triples reader/writer. It supports exactly the three # object shapes this store uses: an IRI object (rdf:type), a plain string @@ -138,12 +158,15 @@ def _render(fields: dict) -> str: lines.append(_lit(subj, P_GROUP, fields["group"])) if fields.get("message_id"): lines.append(_lit(subj, P_MESSAGE_ID, fields["message_id"])) + if fields.get("media"): + lines.append(_lit(subj, P_MEDIA, fields["media"])) return "".join(l + "\n" for l in sorted(lines)) def _parse(text: str) -> dict | None: """Read a message file back into a ``fields`` dict, or None if unparseable.""" - fields: dict = {"delivered": False, "group": None, "message_id": None} + fields: dict = {"delivered": False, "group": None, "message_id": None, + "media": None} subject = None for line in text.splitlines(): line = line.strip() @@ -169,6 +192,8 @@ def _parse(text: str) -> dict | None: fields["text"] = value elif pred == P_MESSAGE_ID: fields["message_id"] = value + elif pred == P_MEDIA: + fields["media"] = value elif pred == P_DELIVERED: fields["delivered"] = value.strip().lower() == "true" if subject is None or "channel" not in fields: @@ -238,6 +263,7 @@ def write_message( message_id: str | None = None, timestamp: float | None = None, delivered: bool = False, + media: str | None = None, ) -> tuple[str, Path]: """Persist one inbound message as a deterministic N-Triples file. @@ -245,6 +271,10 @@ def write_message( message as still owed to triage; pass ``delivered=True`` for a message the gateway is deliberately *not* forwarding (blacklisted, group-blocked or no-action-class) so the daily drain never re-surfaces it. + + ``media`` optionally records a reference (a durable file path) to raw media + retained alongside this message — used by the persist-before-transcribe path + so a voice note survives a failed or crashed STT run. """ ts = time.time() if timestamp is None else float(timestamp) token = secrets.token_hex(8) @@ -258,6 +288,7 @@ def write_message( "message_id": message_id or None, "received_at": _iso(ts), "delivered": bool(delivered), + "media": media or None, } # Filename: zero-padded epoch millis (sortable) + token (unique, IRI-safe). fname = f"{int(ts * 1000):016d}-{token}.nt" @@ -311,5 +342,75 @@ def undelivered( "message_id": fields.get("message_id"), "received_at": fields["received_at"], "text": fields["text"], + "media": fields.get("media"), }) return out + + +def mark_delivered(path: str | Path) -> bool: + """Flip one already-written message's ``delivered`` flag to ``true``. + + This exists for the **persist-before-forward** path: a gateway writes an + inbound message ``delivered = false`` the instant it arrives — before the + gate, before the forward — so that a later failure (a throwing gate, a crash + mid-forward, a killed container) leaves the message on disk for the daily + drain instead of silently dropping it. Once triage actually has the message + (a live forward succeeded, or it was held in a fully-accounted class), the + gateway flips the flag here. + + It performs the same single false→true rewrite as :func:`undelivered`, but + for one known file rather than a scan. Best-effort by design: it returns + ``True`` on success (or if the flag was already ``true``), ``False`` if the + file is missing/unreadable/unparseable, and **never raises** — a bookkeeping + failure must not break message handling. A message left ``false`` by a failed + flip is simply re-surfaced by the next drain (at-least-once), which is the + safe direction. + """ + p = Path(path) + try: + fields = _parse(p.read_text(encoding="utf-8")) + except OSError: + return False + if not fields: + return False + if fields["delivered"]: + return True + fields["delivered"] = True + try: + _atomic_write(_render(fields), p) + except OSError: + return False + return True + + +def update_message(path: str | Path, *, text: str | None = None, + clear_media: bool = False) -> str | None: + """Rewrite a persisted message's mutable fields in place; never raises. + + Used by the **persist-before-transcribe** path: a voice note is written up + front with empty text and a ``media`` reference to its retained audio, then + once STT succeeds this fills in the transcript (``text=…``) and drops the + now-superfluous audio reference (``clear_media=True``). + + Returns the ``media`` value present *before* the call — so a caller clearing + it knows which file to unlink — or ``None`` if there was none or the rewrite + failed. Only the fields named are touched; ``delivered`` and everything else + are preserved. + """ + p = Path(path) + try: + fields = _parse(p.read_text(encoding="utf-8")) + except OSError: + return None + if not fields: + return None + prev_media = fields.get("media") + if text is not None: + fields["text"] = text + if clear_media: + fields["media"] = None + try: + _atomic_write(_render(fields), p) + except OSError: + return None + return prev_media diff --git a/scripts/signal-gateway.py b/scripts/signal-gateway.py index 2ad9f22..9aaaaad 100644 --- a/scripts/signal-gateway.py +++ b/scripts/signal-gateway.py @@ -6,6 +6,8 @@ import mimetypes import os import re +import secrets +import shutil import subprocess import sys import tempfile @@ -201,15 +203,66 @@ def _inbound_gate_decision(sender: str, group_id: str | None) -> dict: def _persist_inbound(question: str, sender: str, group_id: str | None, - delivered: bool) -> None: - """Best-effort persist of one inbound message to the store; never raises.""" + delivered: bool, media: str | None = None): + """Best-effort persist of one inbound message to the store; never raises. + + Returns the store ``Path`` (so the caller can later flip the delivered flag + with :func:`_mark_delivered`) or ``None`` if persistence failed. ``media`` + records a retained raw-audio file for a voice note persisted before + transcription (see :func:`_retain_media`). + """ try: - _ibstore.write_message( + _, path = _ibstore.write_message( INBOUND_STORE_DIR, channel=INBOUND_CHANNEL, sender=sender or "unknown", - text=question, group=group_id or None, delivered=delivered, + text=question, group=group_id or None, delivered=delivered, media=media, ) + return path except Exception as exc: print(f"[signal-gateway] could not persist inbound message: {exc}", flush=True) + return None + + +def _mark_delivered(store_path) -> None: + """Flip a persisted inbound's delivered flag once triage has it; never raises.""" + if store_path is None: + return + try: + _ibstore.mark_delivered(store_path) + except Exception as exc: + print(f"[signal-gateway] could not mark inbound delivered: {exc}", flush=True) + + +def _retain_media(src_path): + """Copy a voice-note attachment into the inbound store's durable media dir. + + signal-cli owns the attachment file it wrote; we copy (not move) it under the + store volume — *before* STT runs — so a failed or crashed transcription can + be retried from a file whose lifetime we control, rather than depending on + signal-cli's own retention. Returns the durable ``Path`` or ``None`` on + failure (the caller then transcribes the original directly). + """ + try: + mdir = _ibstore.media_dir(INBOUND_STORE_DIR) + mdir.mkdir(parents=True, exist_ok=True) + dest = mdir / f"{secrets.token_hex(8)}{Path(src_path).suffix}" + shutil.copy2(str(src_path), str(dest)) + return dest + except Exception as exc: # noqa: BLE001 + print(f"[signal-gateway] could not retain voice-note media: {exc}", flush=True) + return None + + +def _update_inbound(store_path, *, text: str | None = None, + clear_media: bool = False): + """Fill in a pre-persisted message's transcript / drop its media ref; never + raises. Returns the media path that was cleared (to unlink), else None.""" + if store_path is None: + return None + try: + return _ibstore.update_message(store_path, text=text, clear_media=clear_media) + except Exception as exc: # noqa: BLE001 + print(f"[signal-gateway] could not update inbound message: {exc}", flush=True) + return None # Outbound send-control policy — the messenger analogue of EMAIL_SEND_POLICY. @@ -1043,7 +1096,39 @@ def _handle_event(event: dict) -> None: print(f"[signal-gateway] could not record recent sender: {exc}", flush=True) voice, files = _split_attachments(event) - if voice is not None: + # A voice note is persisted BEFORE transcription (never-drop): if the pre- + # persist happened, this holds its store Path so the forward below reuses the + # same record instead of writing a second one. + voice_store_path = None + if voice is not None and SIGNAL_GATEWAY_MODE == "inbox": + print(f"[signal-gateway] processing voice message from {sender}", flush=True) + # Never-drop: retain the audio and persist the message up front, THEN + # transcribe. A failed or crashed STT run leaves a durable, re- + # transcribable record (delivered=False, media set) for the daily drain — + # instead of vanishing at the skip-return below, downstream of where + # _forward_to_inbox persists. Only in inbox mode: a control account has no + # triage drain that would pick a persisted record back up, so the never- + # drop ledger is an inbox-mode concept and persisting there would leak. + durable = _retain_media(voice) or voice + voice_store_path = _persist_inbound( + "", sender, _extract_group_id(event), delivered=False, media=str(durable), + ) + try: + question, lang = _transcribe(durable) + except Exception as exc: # noqa: BLE001 - keep audio for retry + print(f"[signal-gateway] transcription failed for {sender}; " + f"kept for retry: {exc}", flush=True) + question, lang = "", DEFAULT_LANGUAGE + else: + # Transcript in hand: fill it into the record and drop the now- + # redundant retained audio (the text supersedes it). + prev = _update_inbound(voice_store_path, text=question, clear_media=True) + if prev: + Path(prev).unlink(missing_ok=True) + elif voice is not None: + # Control mode: transient handling (no durable spool, no retry) — the + # never-drop ledger is inbox-only. Unchanged from the pre-never-drop path: + # signal-cli owns the attachment file, so it is not unlinked here. print(f"[signal-gateway] processing voice message from {sender}", flush=True) question, lang = _transcribe(voice) else: @@ -1056,6 +1141,12 @@ def _handle_event(event: dict) -> None: if question: print(f"[signal-gateway] processing text message from {sender}", flush=True) if not question and not files: + if voice_store_path is not None: + # A voice note whose transcription failed: not dropped — it is on disk + # (delivered=False, audio retained) for the daily drain / a re-transcribe. + print(f"[signal-gateway] voice note from {sender} not transcribed; " + f"retained for retry (not dropped)", flush=True) + return # Log the raw event structure to help diagnose why content wasn't extracted event_sample = json.dumps(event, default=str) if len(event_sample) > 500: @@ -1068,7 +1159,7 @@ def _handle_event(event: dict) -> None: # account hands it to the user's triage and stays silent towards the sender. if SIGNAL_GATEWAY_MODE == "inbox": _forward_to_inbox(question, lang, sender, group_id=_extract_group_id(event), - files=files) + files=files, store_path=voice_store_path) else: _handle_control_message(question, lang, sender, files=files) @@ -1111,7 +1202,8 @@ def _handle_control_message(question: str, lang: str, sender: str, def _forward_to_inbox(question: str, lang: str, sender: str, group_id: str | None = None, - files: list[dict] | None = None) -> None: + files: list[dict] | None = None, + store_path=None) -> None: """Hand an inbox-account message to the user's triage, notifying the user. The account is one of the user's own message sources, so the message is the @@ -1130,12 +1222,28 @@ def _forward_to_inbox(question: str, lang: str, sender: str, if is_group: sender_label += " [group]" + # Persist FIRST, before any routing decision — the never-drop invariant. + # signal-cli has already drained (acked) this message from the server, so if + # it is lost here it is gone for good. Writing it up front as delivered=False + # means that any later failure — a throwing gate, a crash mid-forward, a + # killed container — leaves the message on disk for the daily drain to catch + # instead of silently dropping it. The flag is flipped to true below once the + # message is actually accounted for (forwarded to triage, or held in a + # fully-resolved class). A voice note was already persisted before + # transcription; reuse that record instead of writing a second one. + if store_path is None: + store_path = _persist_inbound(question, sender, group_id, delivered=False) + # Delivery gate: decide whether this sender is worth a model turn now. A - # held message is persisted (so the daily drain, or plain SPARQL history, - # still sees it) and no `claude -p` session is spawned. + # held message is already persisted above; no `claude -p` session is spawned. gate = _inbound_gate_decision(sender, group_id) if not gate["forward"]: - _persist_inbound(question, sender, group_id, delivered=gate["delivered_if_held"]) + # Mark delivered only for a message that is fully accounted for (a + # blacklisted/no-action class the drain must never re-surface). One held + # merely because the sender is not yet whitelisted stays delivered=False + # so the daily drain still picks it up. + if gate["delivered_if_held"]: + _mark_delivered(store_path) print( f"[signal-gateway] gate held inbox message from {sender_label} " f"({gate['reason']}); no model turn", @@ -1216,10 +1324,13 @@ def _forward_to_inbox(question: str, lang: str, sender: str, except requests.exceptions.RequestException as exc: print(f"[signal-gateway] connection error forwarding inbox message from {sender_label}: {exc}", flush=True) - # Persist AFTER forwarding so the delivered flag reflects reality: a message - # handed to triage is delivered; one whose forward failed stays undelivered - # and the daily drain retries it. - _persist_inbound(question, sender, group_id, delivered=forwarded) + # Flip the persisted message's delivered flag to reflect reality: a message + # handed to triage is delivered; one whose forward failed stays delivered=False + # (as written up front) so the daily drain retries it. At-least-once: a crash + # between the forward and this flip may re-surface the message on the next + # drain, which is the safe direction — a rare duplicate beats a silent loss. + if forwarded: + _mark_delivered(store_path) @@ -1843,7 +1954,15 @@ def main() -> None: if events: print(f"[signal-gateway] received {len(events)} event(s)", flush=True) for event in events: - _handle_event(event) + # Isolate each event: signal-cli has already drained (acked) this + # whole batch, so an exception escaping one _handle_event would + # abort the loop and permanently lose every remaining event in the + # batch. Contain the failure to the one event and keep going. + try: + _handle_event(event) + except Exception as exc: + print(f"[signal-gateway] error handling event: {exc}", flush=True) + print(traceback.format_exc(), flush=True) except subprocess.TimeoutExpired: _note_receive_result(False, "signal-cli timed out") print("[signal-gateway] warning: signal-cli timed out, retrying", flush=True) diff --git a/scripts/telegram-gateway.py b/scripts/telegram-gateway.py index 5a2a225..39b8edc 100644 --- a/scripts/telegram-gateway.py +++ b/scripts/telegram-gateway.py @@ -42,6 +42,8 @@ import mimetypes import os import re +import secrets +import shutil import sys import tempfile import threading @@ -205,15 +207,66 @@ def _inbound_gate_decision(sender: str, group_id: str | None) -> dict: def _persist_inbound(question: str, sender: str, group_id: str | None, - delivered: bool) -> None: - """Best-effort persist of one inbound message to the store; never raises.""" + delivered: bool, media: str | None = None): + """Best-effort persist of one inbound message to the store; never raises. + + Returns the store ``Path`` (so the caller can later flip the delivered flag + with :func:`_mark_delivered`) or ``None`` if persistence failed. ``media`` + records a retained raw-audio file for a voice note persisted before + transcription (see :func:`_retain_media`). + """ try: - _ibstore.write_message( + _, path = _ibstore.write_message( INBOUND_STORE_DIR, channel=INBOUND_CHANNEL, sender=sender or "unknown", - text=question, group=group_id or None, delivered=delivered, + text=question, group=group_id or None, delivered=delivered, media=media, ) + return path except Exception as exc: print(f"[telegram-gateway] could not persist inbound message: {exc}", flush=True) + return None + + +def _mark_delivered(store_path) -> None: + """Flip a persisted inbound's delivered flag once triage has it; never raises.""" + if store_path is None: + return + try: + _ibstore.mark_delivered(store_path) + except Exception as exc: + print(f"[telegram-gateway] could not mark inbound delivered: {exc}", flush=True) + + +def _retain_media(temp_path): + """Move a downloaded media file into the inbound store's durable media dir. + + A voice note is downloaded to a temp file that is otherwise unlinked after + transcription. Retaining it under the store volume — *before* STT runs — is + what lets a failed or crashed transcription be retried instead of the message + vanishing. Returns the durable ``Path`` or ``None`` on failure (the caller + then falls back to transcribing the temp file directly). + """ + try: + mdir = _ibstore.media_dir(INBOUND_STORE_DIR) + mdir.mkdir(parents=True, exist_ok=True) + dest = mdir / f"{secrets.token_hex(8)}{Path(temp_path).suffix}" + shutil.move(str(temp_path), str(dest)) + return dest + except Exception as exc: # noqa: BLE001 + print(f"[telegram-gateway] could not retain voice-note media: {exc}", flush=True) + return None + + +def _update_inbound(store_path, *, text: str | None = None, + clear_media: bool = False): + """Fill in a pre-persisted message's transcript / drop its media ref; never + raises. Returns the media path that was cleared (to unlink), else None.""" + if store_path is None: + return None + try: + return _ibstore.update_message(store_path, text=text, clear_media=clear_media) + except Exception as exc: # noqa: BLE001 + print(f"[telegram-gateway] could not update inbound message: {exc}", flush=True) + return None SEND_APPROVAL_BASE_URL = os.environ.get("SEND_APPROVAL_BASE_URL", "").rstrip("/") @@ -569,17 +622,31 @@ def _inbound_image_files(image_path, image_mime: str | None) -> list[dict]: def _handle_inbound(text: str, lang: str, chat_id: str, sender: str, is_group: bool, sender_name: str | None, - files: list[dict] | None = None) -> None: - """Blocking dispatch — runs in a worker thread, off the asyncio loop.""" + files: list[dict] | None = None, + store_path=None) -> None: + """Blocking dispatch — runs in a worker thread, off the asyncio loop. + + ``store_path`` is set when the caller already persisted this message before + transcription (the never-drop voice-note path): it is threaded to + :func:`_forward_to_inbox` so the forward reuses that record instead of + writing a second one. + """ _record_recent_sender(str(chat_id), sender_name, None, is_group) if not text and not files: - print(f"[telegram-gateway] skipping message from {sender} (no text/audio/image content)", flush=True) + if store_path is not None: + # A voice note whose transcription failed: not dropped — it is on disk + # (delivered=False, audio retained) for the daily drain / a re-transcribe. + print(f"[telegram-gateway] voice note from {sender} not transcribed; " + f"retained for retry (not dropped)", flush=True) + else: + print(f"[telegram-gateway] skipping message from {sender} (no text/audio/image content)", flush=True) return if text and lang == DEFAULT_LANGUAGE: lang = _detect_text_language(text) if TELEGRAM_GATEWAY_MODE == "inbox": _forward_to_inbox(text, lang, str(chat_id), is_group=is_group, - sender_name=sender_name, files=files) + sender_name=sender_name, files=files, + store_path=store_path) else: _handle_control_message(text, lang, str(chat_id), sender, files=files) @@ -638,17 +705,48 @@ async def _on_new_message(event) -> None: def _work(): nonlocal text, lang + # A voice note is persisted BEFORE transcription (never-drop): if the + # pre-persist happened, this holds its store Path so the forward reuses + # the same record instead of writing a second one. + voice_store_path = None if media_path: - try: - print(f"[telegram-gateway] transcribing voice note from {sender}", flush=True) - text, lang = _transcribe(Path(media_path)) - except Exception as exc: # noqa: BLE001 - degrade to placeholder - print(f"[telegram-gateway] transcription failed: {exc}", flush=True) - finally: - Path(media_path).unlink(missing_ok=True) + if TELEGRAM_GATEWAY_MODE == "inbox": + # Never-drop: retain the audio and persist the message up front, + # THEN transcribe. A failed or crashed STT run leaves a durable, + # re-transcribable record (delivered=False, media set) for the + # daily drain — instead of vanishing at the skip-return in + # _handle_inbound, downstream of where _forward_to_inbox + # persists. Only in inbox mode: a control account has no triage + # drain that would pick a persisted record back up. + durable = _retain_media(media_path) or media_path + grp = str(chat_id) if is_group else None + voice_store_path = _persist_inbound( + "", sender, grp, delivered=False, media=str(durable), + ) + try: + print(f"[telegram-gateway] transcribing voice note from {sender}", flush=True) + text, lang = _transcribe(Path(durable)) + except Exception as exc: # noqa: BLE001 - keep audio for retry + print(f"[telegram-gateway] transcription failed for {sender}; " + f"kept for retry: {exc}", flush=True) + else: + # Transcript in hand: fill it into the record and drop the + # now-redundant retained audio (the text supersedes it). + prev = _update_inbound(voice_store_path, text=text, clear_media=True) + if prev: + Path(prev).unlink(missing_ok=True) + else: + # Control mode: transient handling (no durable spool, no retry). + try: + print(f"[telegram-gateway] transcribing voice note from {sender}", flush=True) + text, lang = _transcribe(Path(media_path)) + except Exception as exc: # noqa: BLE001 - degrade to placeholder + print(f"[telegram-gateway] transcription failed: {exc}", flush=True) + finally: + Path(media_path).unlink(missing_ok=True) files = _inbound_image_files(image_path, image_mime) _handle_inbound(text, lang, str(chat_id), sender, is_group, sender_name, - files=files) + files=files, store_path=voice_store_path) _LOOP.run_in_executor(None, _work) except Exception as exc: # noqa: BLE001 - one bad message must not stall the loop @@ -859,8 +957,14 @@ def _handle_control_message(question: str, lang: str, chat_id: str, sender: str, def _forward_to_inbox(question: str, lang: str, chat_id: str, is_group: bool = False, sender_name: str | None = None, - files: list[dict] | None = None) -> None: - """Hand an inbox-account message to the user's triage, notifying the user.""" + files: list[dict] | None = None, + store_path=None) -> None: + """Hand an inbox-account message to the user's triage, notifying the user. + + ``store_path`` is set when the caller already persisted this message before + transcription (the never-drop voice-note path): the persist-first step below + is then skipped so the same record is reused instead of a second one written. + """ sender_label = sender_name or chat_id if sender_name: sender_label = f"{sender_name} ({chat_id})" @@ -873,10 +977,24 @@ def _forward_to_inbox(question: str, lang: str, chat_id: str, handle = str(chat_id) if chat_id else "unknown" group_id = handle if is_group else None + # Persist FIRST, before any routing decision — the never-drop invariant. The + # inbound event has already been consumed from the Telegram session, so if it + # is lost here it is gone for good. Writing it up front as delivered=False + # means any later failure (a throwing gate, a crash mid-forward, a killed + # container) leaves the message on disk for the daily drain instead of + # silently dropping it. The flag is flipped to true below once the message is + # accounted for (forwarded to triage, or held in a fully-resolved class). + if store_path is None: + store_path = _persist_inbound(question, handle, group_id, delivered=False) + # Delivery gate: only whitelisted / unknown senders get a model turn now. gate = _inbound_gate_decision(handle, group_id) if not gate["forward"]: - _persist_inbound(question, handle, group_id, delivered=gate["delivered_if_held"]) + # Mark delivered only for a fully-accounted class (blacklisted/no-action) + # the drain must never re-surface. One held merely for a not-yet- + # whitelisted sender stays delivered=False for the daily drain. + if gate["delivered_if_held"]: + _mark_delivered(store_path) print( f"[telegram-gateway] gate held inbox message from {sender_label} " f"({gate['reason']}); no model turn", @@ -949,9 +1067,12 @@ def _forward_to_inbox(question: str, lang: str, chat_id: str, except requests.exceptions.RequestException as exc: print(f"[telegram-gateway] connection error forwarding inbox message from {sender_label}: {exc}", flush=True) - # Persist AFTER forwarding so the delivered flag reflects reality: a failed - # forward stays undelivered and the daily drain retries it. - _persist_inbound(question, handle, group_id, delivered=forwarded) + # Flip the persisted message's delivered flag: a message handed to triage is + # delivered; a failed forward stays delivered=False (as written up front) so + # the daily drain retries it. At-least-once: a crash between the forward and + # this flip may re-surface the message on the next drain — the safe direction. + if forwarded: + _mark_delivered(store_path) # ── Recent-senders store ────────────────────────────────────────────────────── diff --git a/scripts/whatsapp-gateway.py b/scripts/whatsapp-gateway.py index 3ac4466..94b4468 100644 --- a/scripts/whatsapp-gateway.py +++ b/scripts/whatsapp-gateway.py @@ -40,6 +40,8 @@ import mimetypes import os import re +import secrets +import shutil import tempfile import threading import time @@ -221,15 +223,66 @@ def _inbound_gate_decision(sender: str, group_id: str | None) -> dict: def _persist_inbound(question: str, sender: str, group_id: str | None, - delivered: bool) -> None: - """Best-effort persist of one inbound message to the store; never raises.""" + delivered: bool, media: str | None = None): + """Best-effort persist of one inbound message to the store; never raises. + + Returns the store ``Path`` (so the caller can later flip the delivered flag + with :func:`_mark_delivered`) or ``None`` if persistence failed. ``media`` + records a retained raw-audio file for a voice note persisted before + transcription (see :func:`_retain_media`). + """ try: - _ibstore.write_message( + _, path = _ibstore.write_message( INBOUND_STORE_DIR, channel=INBOUND_CHANNEL, sender=sender or "unknown", - text=question, group=group_id or None, delivered=delivered, + text=question, group=group_id or None, delivered=delivered, media=media, ) + return path except Exception as exc: print(f"[whatsapp-gateway] could not persist inbound message: {exc}", flush=True) + return None + + +def _mark_delivered(store_path) -> None: + """Flip a persisted inbound's delivered flag once triage has it; never raises.""" + if store_path is None: + return + try: + _ibstore.mark_delivered(store_path) + except Exception as exc: + print(f"[whatsapp-gateway] could not mark inbound delivered: {exc}", flush=True) + + +def _retain_media(temp_path): + """Move a downloaded media file into the inbound store's durable media dir. + + The bridge downloads a voice note to a temp file that is otherwise unlinked + after transcription. Retaining it under the store volume — *before* STT runs + — is what lets a failed or crashed transcription be retried instead of the + message vanishing. Returns the durable ``Path`` or ``None`` on failure (the + caller then falls back to transcribing the temp file directly). + """ + try: + mdir = _ibstore.media_dir(INBOUND_STORE_DIR) + mdir.mkdir(parents=True, exist_ok=True) + dest = mdir / f"{secrets.token_hex(8)}{Path(temp_path).suffix}" + shutil.move(str(temp_path), str(dest)) + return dest + except Exception as exc: # noqa: BLE001 + print(f"[whatsapp-gateway] could not retain voice-note media: {exc}", flush=True) + return None + + +def _update_inbound(store_path, *, text: str | None = None, + clear_media: bool = False): + """Fill in a pre-persisted message's transcript / drop its media ref; never + raises. Returns the media path that was cleared (to unlink), else None.""" + if store_path is None: + return None + try: + return _ibstore.update_message(store_path, text=text, clear_media=clear_media) + except Exception as exc: # noqa: BLE001 + print(f"[whatsapp-gateway] could not update inbound message: {exc}", flush=True) + return None # Public base URL used to build approval links returned to the caller. @@ -1276,19 +1329,52 @@ def _handle_message_event(event) -> None: # no-model-turn path anyway, so their media is never downloaded. files = [] if is_broadcast else _inbound_image_files(message) + # A voice note is persisted BEFORE transcription (never-drop): if the pre- + # persist happened, this holds its store Path so the forward below reuses the + # same record instead of writing a second one. + voice_store_path = None if not text and not files: # No text — try a voice note (download + transcribe via the STT service). audio = _extract_audio(message) if audio is not None: media = _download_media(message) if media is not None: - try: - print(f"[whatsapp-gateway] transcribing voice note from {sender}", flush=True) - text, lang = _transcribe(media) - except Exception as exc: # noqa: BLE001 - degrade to placeholder - print(f"[whatsapp-gateway] transcription failed: {exc}", flush=True) - finally: - media.unlink(missing_ok=True) + if is_broadcast or WHATSAPP_GATEWAY_MODE != "inbox": + # Transient handling (no durable spool, no retry): a status + # post is gated to a no-model-turn path anyway, and a + # control-mode account has no triage drain that would ever + # pick a persisted record back up — so persisting here would + # only leak. The never-drop ledger is an inbox-mode concept. + try: + print(f"[whatsapp-gateway] transcribing voice note from {sender}", flush=True) + text, lang = _transcribe(media) + except Exception as exc: # noqa: BLE001 - degrade to placeholder + print(f"[whatsapp-gateway] transcription failed: {exc}", flush=True) + finally: + media.unlink(missing_ok=True) + else: + # Never-drop: retain the audio and persist the message up + # front, THEN transcribe. A failed or crashed STT run leaves a + # durable, re-transcribable record (delivered=False, media set) + # for the daily drain — instead of vanishing at the skip-return + # below, downstream of where _forward_to_inbox persists. + durable = _retain_media(media) or media + grp = _jid_addr(chat_jid) if is_group else None + voice_store_path = _persist_inbound( + "", sender, grp, delivered=False, media=str(durable), + ) + try: + print(f"[whatsapp-gateway] transcribing voice note from {sender}", flush=True) + text, lang = _transcribe(durable) + except Exception as exc: # noqa: BLE001 - keep audio for retry + print(f"[whatsapp-gateway] transcription failed for {sender}; " + f"kept for retry: {exc}", flush=True) + else: + # Transcript in hand: fill it into the record and drop the + # now-redundant audio (the text supersedes it). + prev = _update_inbound(voice_store_path, text=text, clear_media=True) + if prev: + Path(prev).unlink(missing_ok=True) if text and lang == DEFAULT_LANGUAGE: lang = _detect_text_language(text) @@ -1307,7 +1393,13 @@ def _handle_message_event(event) -> None: _record_recent_sender(sender_jid, chat_jid, push_name) if not text and not files: - print(f"[whatsapp-gateway] skipping message from {sender} (no text/audio/image content)", flush=True) + if voice_store_path is not None: + # A voice note whose transcription failed: not dropped — it is on disk + # (delivered=False, audio retained) for the daily drain / a re-transcribe. + print(f"[whatsapp-gateway] voice note from {sender} not transcribed; " + f"retained for retry (not dropped)", flush=True) + else: + print(f"[whatsapp-gateway] skipping message from {sender} (no text/audio/image content)", flush=True) return # The account's mode — not the content — decides handling. @@ -1322,7 +1414,8 @@ def _handle_message_event(event) -> None: # through the normal send-approval policy, so a group send is not silent. origin = _jid_addr(chat_jid) or _jid_addr(sender_jid) _forward_to_inbox(text, lang, sender, is_group=is_group, - sender_name=push_name, origin=origin, files=files) + sender_name=push_name, origin=origin, files=files, + store_path=voice_store_path) else: _handle_control_message(text, lang, sender, files=files) @@ -1360,7 +1453,8 @@ def _handle_control_message(question: str, lang: str, sender: str, def _forward_to_inbox(question: str, lang: str, sender: str, is_group: bool = False, sender_name: str | None = None, origin: str | None = None, - files: list[dict] | None = None) -> None: + files: list[dict] | None = None, + store_path=None) -> None: """Hand an inbox-account message to the user's triage, notifying the user. The account is one of the user's own message sources, so the message is the @@ -1373,6 +1467,10 @@ def _forward_to_inbox(question: str, lang: str, sender: str, embedded in the prompt, so a later reply is addressed by token — back to this same conversation — rather than by re-resolving the sender's name, which can land on the wrong account. + + ``store_path`` is set when the caller already persisted this message before + forwarding (the voice-note persist-before-transcribe path): the record is + reused for the delivered flip instead of writing a second one here. """ sender_label = sender or "unknown" if sender_name: @@ -1384,10 +1482,25 @@ def _forward_to_inbox(question: str, lang: str, sender: str, # group-block policy matches on. For a 1:1 there is no group. group_id = origin if is_group else None + # Persist FIRST, before any routing decision — the never-drop invariant. The + # inbound event has already been consumed from the WhatsApp session, so if it + # is lost here it is gone for good. Writing it up front as delivered=False + # means any later failure (a throwing gate, a crash mid-forward, a killed + # container) leaves the message on disk for the daily drain instead of + # silently dropping it. The flag is flipped to true below once the message is + # accounted for (forwarded to triage, or held in a fully-resolved class). + # A voice note was already persisted before transcription; reuse that record. + if store_path is None: + store_path = _persist_inbound(question, sender, group_id, delivered=False) + # Delivery gate: only whitelisted / unknown senders get a model turn now. gate = _inbound_gate_decision(sender, group_id) if not gate["forward"]: - _persist_inbound(question, sender, group_id, delivered=gate["delivered_if_held"]) + # Mark delivered only for a fully-accounted class (blacklisted/no-action) + # the drain must never re-surface. One held merely for a not-yet- + # whitelisted sender stays delivered=False for the daily drain. + if gate["delivered_if_held"]: + _mark_delivered(store_path) print( f"[whatsapp-gateway] gate held inbox message from {sender_label} " f"({gate['reason']}); no model turn", @@ -1455,9 +1568,12 @@ def _forward_to_inbox(question: str, lang: str, sender: str, except requests.exceptions.RequestException as exc: print(f"[whatsapp-gateway] connection error forwarding inbox message from {sender_label}: {exc}", flush=True) - # Persist AFTER forwarding so the delivered flag reflects reality: a failed - # forward stays undelivered and the daily drain retries it. - _persist_inbound(question, sender, group_id, delivered=forwarded) + # Flip the persisted message's delivered flag: a message handed to triage is + # delivered; a failed forward stays delivered=False (as written up front) so + # the daily drain retries it. At-least-once: a crash between the forward and + # this flip may re-surface the message on the next drain — the safe direction. + if forwarded: + _mark_delivered(store_path) def _forward_status_to_inbox(text: str, lang: str, sender: str, diff --git a/tests/test_inbound_store.py b/tests/test_inbound_store.py index f6f440f..c269bf9 100644 --- a/tests/test_inbound_store.py +++ b/tests/test_inbound_store.py @@ -70,6 +70,27 @@ def test_undelivered_drains_once(): print("PASS test_undelivered_drains_once") +def test_mark_delivered_roundtrip(): + with tempfile.TemporaryDirectory() as tmp: + # Persist-before-forward: two messages land delivered=false. + _, path_a = ist.write_message(tmp, channel="signal", sender="a", + text="accounted", timestamp=10.0) + ist.write_message(tmp, channel="signal", sender="b", + text="failed-forward", timestamp=20.0) + # "a" was actually accounted for (forward succeeded) → flip it. + assert ist.mark_delivered(path_a) is True + # Idempotent: a second flip on an already-true message still reports True + # and does not re-write it into the drain. + assert ist.mark_delivered(path_a) is True + # The drain now surfaces only "b" — "a" is no longer owed, while "b", + # left delivered=false (its forward failed), is still surfaced. + got = ist.undelivered(tmp) + assert [m["text"] for m in got] == ["failed-forward"] + # Safe on a missing file: returns False, never raises. + assert ist.mark_delivered(ist.messages_dir(tmp) / "nope.nt") is False + print("PASS test_mark_delivered_roundtrip") + + def test_since_filter(): with tempfile.TemporaryDirectory() as tmp: ist.write_message(tmp, channel="wa", sender="a", text="old", timestamp=100.0) @@ -111,6 +132,61 @@ def test_group_and_optional_fields(): print("PASS test_group_and_optional_fields") +def test_media_roundtrip_and_undelivered(): + with tempfile.TemporaryDirectory() as tmp: + # A voice note persisted before transcription: empty text, a media ref. + subj, path = ist.write_message( + tmp, channel="whatsapp", sender="a", text="", + media="/data/media/deadbeef.ogg", timestamp=1.0, + ) + fields = ist._parse(path.read_text(encoding="utf-8")) + assert fields["media"] == "/data/media/deadbeef.ogg" + assert fields["text"] == "" + assert fields["delivered"] is False + # media survives the drain and is handed to the caller. + got = ist.undelivered(tmp) + assert len(got) == 1 + assert got[0]["media"] == "/data/media/deadbeef.ogg" + assert got[0]["text"] == "" + print("PASS test_media_roundtrip_and_undelivered") + + +def test_update_message_fills_transcript_and_clears_media(): + with tempfile.TemporaryDirectory() as tmp: + subj, path = ist.write_message( + tmp, channel="signal", sender="a", text="", + media="/data/media/abc.ogg", timestamp=1.0, + ) + # Transcription succeeded: fill the text, clear the media ref, learn which + # file to unlink (the prior media value). + prev = ist.update_message(path, text="hallo welt", clear_media=True) + assert prev == "/data/media/abc.ogg" + fields = ist._parse(path.read_text(encoding="utf-8")) + assert fields["text"] == "hallo welt" + assert fields["media"] is None + # delivered flag is untouched by an update. + assert fields["delivered"] is False + # A second clear finds nothing left to unlink. + assert ist.update_message(path, clear_media=True) is None + print("PASS test_update_message_fills_transcript_and_clears_media") + + +def test_update_message_missing_file_returns_none(): + with tempfile.TemporaryDirectory() as tmp: + assert ist.update_message(Path(tmp) / "nope.nt", text="x") is None + print("PASS test_update_message_missing_file_returns_none") + + +def test_write_without_media_has_no_media_predicate(): + with tempfile.TemporaryDirectory() as tmp: + _, path = ist.write_message(tmp, channel="tg", sender="a", text="plain", + timestamp=1.0) + text = path.read_text(encoding="utf-8") + assert ist.P_MEDIA not in text + assert ist._parse(text)["media"] is None + print("PASS test_write_without_media_has_no_media_predicate") + + def test_missing_dir_is_empty(): with tempfile.TemporaryDirectory() as tmp: assert ist.undelivered(Path(tmp) / "nope") == [] @@ -131,9 +207,14 @@ def test_since_epoch_and_iso_equivalent(): if __name__ == "__main__": test_write_and_roundtrip() test_undelivered_drains_once() + test_mark_delivered_roundtrip() test_since_filter() test_persist_but_not_forward() test_group_and_optional_fields() + test_media_roundtrip_and_undelivered() + test_update_message_fills_transcript_and_clears_media() + test_update_message_missing_file_returns_none() + test_write_without_media_has_no_media_predicate() test_missing_dir_is_empty() test_since_epoch_and_iso_equivalent() print("all inbound_store tests passed")