fix(signal): FIFO-evict the quote-detection timestamp cache

`_sent_message_timestamps` (the reply-to-own-message quote cache) used a
`set` evicted with `set.pop()`, which removes an ARBITRARY element — so once
more than the cap (500) outbound timestamps are tracked, a still-recent
timestamp could be dropped while older ones survive, missing a genuine
reply-to-own-message. Convert it to an OrderedDict with FIFO (oldest-first)
eviction, mirroring the recently-hardened echo ring (#31250). This closes the
same bug class on the sibling cache.

Adds a regression test asserting oldest-first eviction + MRU promotion.
This commit is contained in:
kshitijk4poor 2026-06-20 21:00:46 +05:30
parent 85ad7c9b0a
commit 26d9a3c710
2 changed files with 30 additions and 4 deletions

View file

@ -318,7 +318,10 @@ class SignalAdapter(BasePlatformAdapter):
# Signal quote.id is the timestamp of the quoted message, so this lets
# inbound replies identify that the user replied to a message sent by
# this bot even after the self-sync echo was filtered above.
self._sent_message_timestamps: set[str] = set()
# OrderedDict (not set) so the cap evicts the OLDEST timestamp in FIFO
# order — a plain set.pop() removes an arbitrary element, which could
# drop a still-recent timestamp and miss a genuine reply-to-own-message.
self._sent_message_timestamps: "OrderedDict[str, None]" = OrderedDict()
self._max_sent_message_timestamps = 500
# Signal increasingly exposes ACI/PNI UUIDs as stable recipient IDs.
# Keep a best-effort mapping so outbound sends can upgrade from a
@ -807,9 +810,14 @@ class SignalAdapter(BasePlatformAdapter):
"""Keep a bounded cache of outbound Signal timestamps for quote matching."""
if timestamp is None:
return
self._sent_message_timestamps.add(str(timestamp))
if len(self._sent_message_timestamps) > self._max_sent_message_timestamps:
self._sent_message_timestamps.pop()
key = str(timestamp)
# Re-insert to mark most-recently-used so eviction drops genuinely old
# timestamps, not a recently re-seen one.
self._sent_message_timestamps.pop(key, None)
self._sent_message_timestamps[key] = None
# FIFO-evict the oldest entry once over the cap.
while len(self._sent_message_timestamps) > self._max_sent_message_timestamps:
self._sent_message_timestamps.popitem(last=False)
def _extract_contact_uuid(self, contact: Any, phone_number: str) -> Optional[str]:
"""Best-effort extraction of a Signal service ID from listContacts output."""

View file

@ -1580,6 +1580,24 @@ class TestSignalQuoteExtraction:
assert "111222333" in adapter._sent_message_timestamps
assert adapter._quote_references_own_message("111222333", None) is True
def test_sent_message_timestamps_evicts_oldest_first(self, monkeypatch):
"""Over the cap, the OLDEST quote-cache timestamp is dropped (FIFO),
not an arbitrary one so a recent reply-to-own-message is still
detected after a burst of sends."""
adapter = _make_signal_adapter(monkeypatch)
adapter._max_sent_message_timestamps = 3
for ts in (1, 2, 3):
adapter._remember_sent_message_timestamp(ts)
# Adding a 4th evicts the oldest (1), keeps the rest in order.
adapter._remember_sent_message_timestamp(4)
assert list(adapter._sent_message_timestamps.keys()) == ["2", "3", "4"]
assert "1" not in adapter._sent_message_timestamps
# Re-seeing an existing ts promotes it so it survives the next eviction.
adapter._remember_sent_message_timestamp(2) # 2 -> most recent
adapter._remember_sent_message_timestamp(5) # evicts oldest (now 3)
assert list(adapter._sent_message_timestamps.keys()) == ["4", "2", "5"]
assert "3" not in adapter._sent_message_timestamps
@pytest.mark.asyncio
async def test_handle_envelope_without_quote_leaves_reply_fields_none(self, monkeypatch):
adapter = _make_signal_adapter(monkeypatch)