Antispam: a dead engine marked the account bootstrapped anyway

`_teach` signalled failure by negating the count taught so far. That
works for every case but the one that matters: when the FIRST message is
refused nothing has been taught yet, and `-0` is `0` — not negative. The
caller's `< 0` test then read a total engine outage as a clean run of
zero, recorded `_bootstrapped` for the account, and the hourly sweep
skipped it from then on. The account most in need of a warm start was
the one guaranteed never to get one.

Which is precisely what happened alongside the missing controller
password: every learn was refused 401, and each account was quietly
retired on its first sweep.

Return None for "the engine stopped accepting these" so failure cannot
be confused with a count, and decline to mark an account whose samples
were all unreadable.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018SqJYt7jD7oUEkoY68sBZo

backend/tests/plugins/test_antispam_learning.py

@@ -330,6 +330,104 @@ async def test_one_failing_account_does_not_stop_the_sweep(
     assert done == [str(accounts[0][1]), str(accounts[2][1])]
 
 
+@pytest.mark.asyncio
+async def test_a_dead_engine_never_marks_the_account_bootstrapped(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """An engine that refuses the FIRST message must not look like a clean run.
+
+    This shipped: failure was signalled by negating the count taught so far, so
+    a refusal on message one returned `-0` — which is `0`, not negative. The
+    caller read that as "learned nothing, fine", marked the account done, and
+    the hourly sweep then skipped it forever. The account most in need of a warm
+    start was the one guaranteed never to get one.
+    """
+    marked: list[object] = []
+
+    async def _settings(_u: object) -> policy.Settings:
+        return SETTINGS
+
+    class _Kv:
+        async def get(self, _k: str, _d: object = None) -> list[str]:
+            return []
+
+        async def set(self, _k: str, v: object) -> None:
+            marked.append(v)
+
+    class _M:
+        async def sample(self, *_a: object, **kw: object) -> list[uuid.UUID]:
+            n = 50 if kw.get("special_use") == "junk" else 50
+            return [uuid.uuid4() for _ in range(n)]
+
+        async def raw(self, *_a: object, **_k: object) -> bytes:
+            return b"x"
+
+    class _Fake:
+        messages = _M()
+
+        def kv(self, _u: object) -> _Kv:
+            return _Kv()
+
+        async def publish(self, *_a: object, **_k: object) -> None:
+            """Present so this test fails on its assertion, not on a bare fake."""
+
+    async def _dead(_raw: bytes, **_kw: object) -> bool:
+        return False  # every learn refused, from the very first one
+
+    monkeypatch.setattr(policy, "load_settings", _settings)
+    monkeypatch.setattr(jobs, "_SDK", _Fake())
+    monkeypatch.setattr(jobs.rspamd, "learn", _dead)
+
+    await jobs.bootstrap_account({}, user_id=str(uuid.uuid4()), account_id=str(uuid.uuid4()))
+    assert marked == [], "marked the account bootstrapped after learning nothing"
+
+
+@pytest.mark.asyncio
+async def test_an_engine_that_dies_midway_also_leaves_the_account_unmarked(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """The case the original sentinel did handle — keep it handled."""
+    marked: list[object] = []
+    calls = {"n": 0}
+
+    async def _settings(_u: object) -> policy.Settings:
+        return SETTINGS
+
+    class _Kv:
+        async def get(self, _k: str, _d: object = None) -> list[str]:
+            return []
+
+        async def set(self, _k: str, v: object) -> None:
+            marked.append(v)
+
+    class _M:
+        async def sample(self, *_a: object, **_kw: object) -> list[uuid.UUID]:
+            return [uuid.uuid4() for _ in range(50)]
+
+        async def raw(self, *_a: object, **_k: object) -> bytes:
+            return b"x"
+
+    class _Fake:
+        messages = _M()
+
+        def kv(self, _u: object) -> _Kv:
+            return _Kv()
+
+        async def publish(self, *_a: object, **_k: object) -> None:
+            """As above."""
+
+    async def _dies(_raw: bytes, **_kw: object) -> bool:
+        calls["n"] += 1
+        return calls["n"] <= 5
+
+    monkeypatch.setattr(policy, "load_settings", _settings)
+    monkeypatch.setattr(jobs, "_SDK", _Fake())
+    monkeypatch.setattr(jobs.rspamd, "learn", _dies)
+
+    await jobs.bootstrap_account({}, user_id=str(uuid.uuid4()), account_id=str(uuid.uuid4()))
+    assert marked == [], "marked the account bootstrapped after a partial run"
+
+
 def test_compose_gives_the_app_the_controller_password() -> None:
     """The learning loop needs the controller password; classification does not.
 
@@ -354,7 +452,9 @@ def test_compose_gives_the_app_the_controller_password() -> None:
         assert anchor in text, f"{rel}: shared env block moved; update this test"
         start = text.index(anchor)
         # The shared block runs to the next top-level key.
-        block = text[start : text.index("\nservices:", start) if "\nservices:" in text[start:] else len(text)]
+        tail = text[start:]
+        end = start + (tail.index("\nservices:") if "\nservices:" in tail else len(tail))
+        block = text[start:end]
         assert "RSPAMD_CONTROLLER_PASSWORD" in block, (
             f"{rel}: the app containers do not receive RSPAMD_CONTROLLER_PASSWORD — "
             "antispam will classify but never learn"

plugins/webmail_plugin_antispam/webmail_plugin_antispam/jobs.py

@@ -51,7 +51,15 @@ MAX_HAM_RATIO = 3
 _DONE_KEY = "_bootstrapped"
 
 
-async def _teach(user_id: uuid.UUID, ids: list[uuid.UUID], *, spam: bool) -> int:
+async def _teach(user_id: uuid.UUID, ids: list[uuid.UUID], *, spam: bool) -> int | None:
+    """Number of messages learned, or None if the engine stopped accepting them.
+
+    None rather than a negative count: the count was once negated to signal
+    failure, which works for every case except the one that matters. If the
+    FIRST message is refused, nothing has been taught yet, `-0` is `0`, and the
+    caller's `< 0` test reads a total engine outage as a clean run of zero — then
+    marks the account bootstrapped, so it is never retried.
+    """
     taught = 0
     for message_id in ids:
         raw = await _SDK.messages.raw(user_id, message_id)
@@ -60,7 +68,7 @@ async def _teach(user_id: uuid.UUID, ids: list[uuid.UUID], *, spam: bool) -> int
         if not await rspamd.learn(raw, user_key=str(user_id), spam=spam):
             # Engine gone. Stop rather than grind through hundreds of failures;
             # the account stays unmarked, so a later run picks it up again.
-            return -taught
+            return None
         taught += 1
         await asyncio.sleep(PAUSE_SECONDS)
     return taught
@@ -106,17 +114,24 @@ async def bootstrap_account(
     ham_ids = ham_ids[: len(spam_ids) * MAX_HAM_RATIO]
 
     spam_taught = await _teach(uid, spam_ids, spam=True)
-    if spam_taught < 0:
+    if spam_taught is None:
         logger.warning(
             "antispam bootstrap for %s stopped: engine unavailable", account_id
         )
         return
     ham_taught = await _teach(uid, ham_ids, spam=False)
-    if ham_taught < 0:
+    if ham_taught is None:
         logger.warning(
             "antispam bootstrap for %s stopped: engine unavailable", account_id
         )
         return
+    if spam_taught == 0:
+        # Every sample was unreadable. Nothing was learned, so do not record a
+        # warm start that did not happen.
+        logger.warning(
+            "antispam bootstrap for %s: no samples could be read", account_id
+        )
+        return
 
     await _SDK.kv(uid).set(_DONE_KEY, [*done, account_id])
     logger.info(