diff --git a/src/builtin_actions.py b/src/builtin_actions.py index 727d2699d..ca5e5158f 100644 --- a/src/builtin_actions.py +++ b/src/builtin_actions.py @@ -1125,14 +1125,14 @@ async def action_learn_sender_signatures(owner: str, **kwargs) -> Tuple[str, boo conn = _imap_connect(None, owner=owner) try: conn.select("INBOX", readonly=True) - status, data = conn.search(None, "ALL") + status, data = conn.uid("SEARCH", None, "ALL") if status != "OK" or not data or not data[0]: return results uids = data[0].split()[-300:][::-1] # newest 300 for uid in uids: try: - st, msg_data = conn.fetch( - uid, "(BODY.PEEK[HEADER.FIELDS (FROM)])" + st, msg_data = conn.uid( + "FETCH", uid, "(BODY.PEEK[HEADER.FIELDS (FROM)])" ) if st != "OK" or not msg_data or not msg_data[0]: continue @@ -1214,7 +1214,7 @@ async def action_learn_sender_signatures(owner: str, **kwargs) -> Tuple[str, boo conn2.select("INBOX", readonly=True) for mm in _msgs: try: - st, data = conn2.fetch(mm["uid"], "(BODY.PEEK[TEXT])") + st, data = conn2.uid("FETCH", mm["uid"], "(BODY.PEEK[TEXT])") if st != "OK" or not data or not data[0]: continue raw = data[0][1] if isinstance(data[0], tuple) else None @@ -1356,13 +1356,13 @@ async def action_daily_brief(owner: str, **kwargs) -> Tuple[str, bool]: conn = _imap_connect(None) try: conn.select("INBOX", readonly=True) - status, data = conn.search(None, "UNSEEN") + status, data = conn.uid("SEARCH", None, "UNSEEN") uids = (data[0].split() if status == "OK" and data and data[0] else []) unread_count = len(uids) # Grab headers for the most recent 5 unread (UIDs increase with arrival) for uid in uids[-5:][::-1]: try: - _, msg_data = conn.fetch(uid, "(BODY.PEEK[HEADER.FIELDS (FROM SUBJECT)])") + _, msg_data = conn.uid("FETCH", uid, "(BODY.PEEK[HEADER.FIELDS (FROM SUBJECT)])") if not msg_data or not msg_data[0]: continue hdr = msg_data[0][1] if isinstance(msg_data[0], tuple) else msg_data[0] diff --git a/tests/test_builtin_actions_owner_scope.py b/tests/test_builtin_actions_owner_scope.py index d14a94462..70e2e389f 100644 --- a/tests/test_builtin_actions_owner_scope.py +++ b/tests/test_builtin_actions_owner_scope.py @@ -109,10 +109,9 @@ async def test_learn_sender_signatures_resolves_llm_for_task_owner(monkeypatch): def select(self, *_args, **_kwargs): return "OK", [] - def search(self, *_args, **_kwargs): - return "OK", [b"1 2 3"] - - def fetch(self, _uid, _query): + def uid(self, command, *_args): + if command == "SEARCH": + return "OK", [b"1 2 3"] return "OK", [(None, b"From: Writer \r\n\r\n")] def logout(self): @@ -171,11 +170,10 @@ async def test_learn_sender_signatures_writes_owner_scoped_cache(monkeypatch, tm def select(self, *_args, **_kwargs): return "OK", [] - def search(self, *_args, **_kwargs): - return "OK", [b"1 2 3"] - - def fetch(self, uid, query): - if "HEADER.FIELDS" in query: + def uid(self, command, uid=None, query=None): + if command == "SEARCH": + return "OK", [b"1 2 3"] + if query and "HEADER.FIELDS" in query: return "OK", [(None, b"From: Writer \r\n\r\n")] return "OK", [ ( diff --git a/tests/test_imap_uid_commands.py b/tests/test_imap_uid_commands.py new file mode 100644 index 000000000..bc4e9401d --- /dev/null +++ b/tests/test_imap_uid_commands.py @@ -0,0 +1,108 @@ +"""Regression: IMAP calls must use uid() not search()/fetch(). + +conn.search() / conn.fetch() operate on volatile positional sequence +numbers that shift whenever messages are deleted or expunged. The +sig-learner and daily-brief actions must use conn.uid("SEARCH", ...) +and conn.uid("FETCH", ...) which address messages by their persistent +RFC 3501 UID (§2.3.1.1, §6.4.8). +""" +import pytest + + +class _SpyImap: + """IMAP stub that records uid() calls and raises on search()/fetch().""" + + def __init__(self, uid_list=b"1 2 3"): + self._uid_list = uid_list + self.uid_calls: list[tuple] = [] + + def select(self, *args, **kwargs): + return "OK", [] + + def uid(self, command, *args): + self.uid_calls.append((command,) + args) + if command == "SEARCH": + return "OK", [self._uid_list] + if command == "FETCH": + query = args[1] if len(args) > 1 else "" + if "HEADER.FIELDS" in query: + return "OK", [(None, b"From: Writer \r\n" + b"Subject: Hello\r\n\r\n")] + return "OK", [(None, b"Body text\r\n\r\nRegards,\r\nThe Writer\r\n")] + return "OK", [] + + def search(self, *args): + raise AssertionError("conn.search() called — must use conn.uid('SEARCH', ...) instead") + + def fetch(self, *args): + raise AssertionError("conn.fetch() called — must use conn.uid('FETCH', ...) instead") + + def logout(self): + pass + + +@pytest.mark.asyncio +async def test_sig_learner_uses_uid_search(monkeypatch): + """_pull_headers must call conn.uid('SEARCH', ...) not conn.search().""" + from routes import email_helpers + from src import task_endpoint + from src.builtin_actions import action_learn_sender_signatures + + spy = _SpyImap() + monkeypatch.setattr(email_helpers, "_imap_connect", lambda *a, **kw: spy) + monkeypatch.setattr(task_endpoint, "resolve_task_candidates", lambda *a, **kw: []) + + message, ok = await action_learn_sender_signatures("alice") + + assert ok is False # no LLM candidates — stops before LLM, after IMAP + assert any(c[0] == "SEARCH" for c in spy.uid_calls), "uid('SEARCH', ...) was not called" + + +@pytest.mark.asyncio +async def test_sig_learner_uses_uid_fetch(monkeypatch): + """_pull_headers must call conn.uid('FETCH', ...) not conn.fetch().""" + from routes import email_helpers + from src import task_endpoint + from src.builtin_actions import action_learn_sender_signatures + + spy = _SpyImap() + monkeypatch.setattr(email_helpers, "_imap_connect", lambda *a, **kw: spy) + monkeypatch.setattr(task_endpoint, "resolve_task_candidates", lambda *a, **kw: []) + + await action_learn_sender_signatures("alice") + + assert any(c[0] == "FETCH" for c in spy.uid_calls), "uid('FETCH', ...) was not called" + + +@pytest.mark.asyncio +async def test_daily_brief_uses_uid_commands(monkeypatch): + """action_daily_brief email section must use uid() not search()/fetch().""" + from core import database + from core import auth as _auth_mod + from routes import email_helpers + from src.builtin_actions import action_daily_brief + + class _Q: + def filter(self, *a, **kw): return self + def join(self, *a, **kw): return self + def order_by(self, *a): return self + def all(self): return [] + + class _Db: + def query(self, *a): return _Q() + def close(self): pass + + class _FakeAuth: + is_configured = False + + monkeypatch.setattr(database, "SessionLocal", _Db) + monkeypatch.setattr(_auth_mod, "AuthManager", lambda: _FakeAuth()) + + spy = _SpyImap(uid_list=b"10 20 30") + monkeypatch.setattr(email_helpers, "_imap_connect", lambda *a, **kw: spy) + + message, ok = await action_daily_brief("") + + assert ok is True + assert any(c[0] == "SEARCH" for c in spy.uid_calls), "uid('SEARCH', ...) was not called" + assert any(c[0] == "FETCH" for c in spy.uid_calls), "uid('FETCH', ...) was not called"