fix: never silently drop an inbound message (persist-before-forward)

[retog]

Problem

A Signal message from a known contact arrived in the Signal app but never surfaced on the dashboard — and left no trace anywhere: not in triage, not in the daily drain, not in the SPARQL inbound history that the delivery ledger is meant to guarantee. The sender still appeared in recent-chats (that store is written first, in its own try/except), which is exactly why the loss was invisible.

Root cause: every inbox-mode gateway persisted each inbound message only at the end of _forward_to_inbox, after the delivery gate, the reply-token mint, the prompt build and the forward POST. Anything that threw before that final write dropped the message. On Signal there is a second, structural amplifier: signal-cli receive drains and acks a whole batch, then the loop iterated _handle_event(event) inside a single try/except — so one throwing event aborted the loop and permanently lost every remaining event in the already-acked batch.

inbound_store.py's own module docstring already promised the opposite: "persisted here ... before any routing decision is made." This PR makes that true.

Fix

• inbound_store.py — add mark_delivered(path): a best-effort false → true flip of one already-written message, mirroring the flip undelivered already performs. Stdlib-only, never raises.
• signal / whatsapp / telegram gateways — persist first as delivered=false the instant a message arrives, capture the path, then mark_delivered once the message is actually accounted for (forwarded to triage, or held in a fully-resolved blacklist/no-action class). A failed forward stays delivered=false, so the daily drain retries it.
• signal-gateway.py — wrap the per-event _handle_event call in its own try/except so one bad event cannot abort the rest of the drained batch. WhatsApp (@client.event) and Telegram (Telethon add_event_handler) already dispatch per-event and were left as-is.

Tradeoff (explicit)

At-least-once. A crash in the narrow window between the persist and the delivered-flip may re-surface a message on the next daily drain — a rare duplicate. That is the safe direction: a duplicate the user sees beats a silent loss they never learn about.

Testing

• py_compile on all four files.
• Round-trip test of inbound_store: a delivered=false write is drained by undelivered; after mark_delivered it is not; mark_delivered is idempotent and safe on a missing file; a message left delivered=false is still surfaced by the drain.

Closes #122

---

Follow-up: voice notes were still dropped (extension)

The persist-first fix above lives inside _forward_to_inbox. A voice note whose transcription fails never reaches it: STT runs in the handler, and on failure text stays empty, so the message hits the "no text/audio/image content" skip-return upstream of _forward_to_inbox. The message was still lost — only the recent-sender entry survived. A timed-out or crashed STT run (or a container killed mid-transcribe) drops the note silently, exactly the failure class this PR set out to close.

Fix

Persist the raw message and its retained audio before transcription:

• inbound_store.py — new optional media field (kb:media, P_MEDIA) recording a retained raw-audio file path. update_message(path, text=…, clear_media=…) fills in the transcript and clears the media ref in place once STT succeeds, returning the prior media path so the caller can unlink it; never raises. undelivered() now surfaces media; new media_dir() helper for the durable audio subdir.
• signal / whatsapp / telegram gateways — in inbox mode, retain the voice-note audio under the store volume and persist the message delivered=false with a media ref up front, then transcribe. On success: fill the transcript, clear + unlink the audio. On failure/crash: the durable delivered=false record with its retained audio stays for the daily drain and re-transcription — instead of vanishing at the skip-return. The pre-persisted store_path is threaded through _forward_to_inbox so no second record is written.
• The never-drop persist is gated to inbox mode: a control account has no triage drain, so persisting there would only leak. Control mode keeps the transient transcribe-then-discard path unchanged.

Testing (extension)

• py_compile on all four files; existing gateway tests (test_inbound_image_forward, test_telegram_send_policy) still pass.
• Added round-trip tests: the media field writes/parses and is surfaced by undelivered; update_message fills the transcript, clears the media ref (returning the prior path), leaves delivered untouched, and is safe on a missing file; a message written without media carries no P_MEDIA predicate.

[aros-agent]

Written by Aros, the project's AI agent, from my own account @aros-agent.

Reviewed within the hour of opening (bet-5 practice: verify the checkable claims). Pulled the branch and checked each one against the diff rather than the description:

• Persist-before-forward is real in all three gateways: store_path = _persist_inbound(..., delivered=False) now runs before the delivery gate in _forward_to_inbox, and the later _mark_delivered(store_path) calls are gated correctly — only on gate["delivered_if_held"] for a held message, only on forwarded for a live one. write_message's existing (subject_uri, path) return (unchanged by this PR) matches the new _, path = _ibstore.write_message(...) unpacking, so that isn't a silent break.
• The batch-abort fix is real and matches the pre-PR bug as described. Confirmed on main: the receive loop was for event in events: _handle_event(event) inside one try/except, so a throwing event did abort the rest of an already-acked batch. The PR wraps each call individually; traceback is already imported.
• WhatsApp/Telegram needed no batch fix, and that's correct too — both dispatch per-event through their own client (@client.event(MessageEv), add_event_handler), never a shared loop over a drained list, so there's no batch to abort.
• py_compile clean on all four files.

One gap, not blocking but worth naming given #114's near-miss on stale tests two days ago: the "Testing" section describes a round-trip test of mark_delivered (drains-once, idempotent, safe on a missing file) — but git diff --stat shows only the four implementation files changed. grep -rl mark_delivered across the repo finds it in zero test files; tests/test_inbound_store.py (the file CI's for t in tests/test_*.py glob actually runs) still has no test_.*mark_delivered case. So the property the whole at-least-once guarantee depends on — that a duplicate surfaces on crash but a delivered message never re-drains — was verified once by hand and isn't checked again by anything that runs. Suggest folding the described round-trip into tests/test_inbound_store.py before or shortly after merge, same shape as test_undelivered_drains_once.

[aros-agent]

Written by Aros, the project's AI agent, from my own account @aros-agent.

Reviewed the new commit (d3a11b7, the voice-note extension) the same way as the first — pulled the branch, ran the tests, read the diff against the claims.

Checks out: the gap it closes is real and matches the description — on main before this PR, a voice note's _transcribe failure hits the "no text/audio/image content" skip-return in _handle_event, which runs upstream of _forward_to_inbox (where the delivered=False record gets written), so a failed transcript left literally nothing, not even the media. _retain_media + up-front write_message(..., media=...) closes that. py_compile clean on all four touched files; python3 tests/test_inbound_store.py — 11/11 pass including the four new ones. The inbox-mode gate (SIGNAL_GATEWAY_MODE == "inbox") is correctly on the new branch only; control mode is untouched, matching the stated "no triage drain, would only leak" reasoning.

One narrow defect, Signal-specific. _retain_media's fallback on copy failure is durable = _retain_media(voice) or voice — and voice there is not a gateway-owned temp file, it's whatever _attachment_path() resolved inside ATTACHMENT_SEARCH_DIRS (SIGNAL_DATA_DIR/attachments or SIGNAL_DATA_DIR itself, i.e. signal-cli's own state dir). If the copy raises (disk full, permission error — the exact class this whole PR exists to survive) and the subsequent transcription then succeeds, _update_inbound(..., clear_media=True) returns that same signal-cli-owned path as prev, and the caller does Path(prev).unlink(missing_ok=True) — deleting a file inside signal-cli's own data directory. Two lines further down, the control-mode branch says explicitly "signal-cli owns the attachment file, so it is not unlinked here" — the retain-failure fallback in the inbox-mode branch above it doesn't honor that.

WhatsApp and Telegram don't have this: their _retain_media docstrings say the pre-retain file is already a self-downloaded temp file the gateway was going to unlink anyway (media_path/media, not an external process's storage), so the same fallback-then-unlink there is fine.

Narrow — needs a retain failure and a subsequent transcription success, so probably rare in practice — but it's the one path in this diff that can delete data the gateway doesn't own, which is exactly the class of thing #122 was about. Suggest either: skip the unlink when durable is voice (i.e. retain never happened), or have _retain_media's caller track whether the copy actually succeeded rather than inferring it from path equality.

Separately, status note rather than a new finding: the mark_delivered round-trip test gap I flagged on the first commit is still open — this commit didn't touch tests/test_inbound_store.py's coverage of it, and that's fine, it's a different fix.