Durable result delivery: OUTBOX + idempotent INBOX so results never die

An 8h search result must survive a transient bot outage, an api/address
misroute, or a restart of either side. Make the librarian->bot result
path durably at-least-once with idempotent rendering:

Shared: durable_queue.DiskQueue - a dependency-free, atomically-written,
one-file-per-key disk queue (unit-tested), shared by both images
(added to Dockerfile.librarian; the bot already COPYs *.py).

Librarian (sender): finished results go to a persistent OUTBOX before
sending; delivery retries with backoff; an entry is removed only on a
positive ACK; a resender thread keeps flushing the OUTBOX, so a result
survives a bot outage AND a librarian restart (OUTBOX is on the state
volume) - it simply keeps trying until acked.

Bot (receiver): /conjurer is now idempotent and durable - each result is
persisted to an INBOX before acking and only queued if its uuid was not
already delivered (dropped as a duplicate) or already pending. Once the
cog actually renders it, mark_delivered() records the uuid and clears the
inbox, so the librarian's resends become no-ops. On startup the bot
replays any accepted-but-unrendered result from the INBOX, so a bot crash
mid-flight doesn't lose it. Pongs stay ephemeral.

Together: the librarian keeps a result until the bot confirms it; the bot
keeps it until it is on screen; duplicates never double-render. Combined
with the deploy return-path fix, an expensive result no longer vanishes.

Tests: unit test_durable_queue; integration test_librarian_outbox
(retry/backoff, resend survives outage) and test_result_durable_delivery
(persist, dedup pending, dedup delivered, replay, pong not persisted).
Suite: 55 unit + 39 integration green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

tests/integration/test_librarian_outbox.py

@@ -0,0 +1,99 @@
+"""Integration: the librarian's durable result OUTBOX + retrying delivery.
+
+An 8-hour search result must not be lost to a transient bot outage. The result
+is written to the OUTBOX before sending; delivery retries with backoff; the
+entry is removed only on a positive ACK; and the resender keeps flushing the
+OUTBOX (across restarts, since it is on the persistent state volume).
+"""
+import logging
+import sys
+import types
+
+import pytest
+
+# conjurer_librarian imports `from habanero import Crossref` at import time; the
+# integration job doesn't install habanero. Stub it (we never build a real
+# Librarian here).
+if "habanero" not in sys.modules:
+    _habanero = types.ModuleType("habanero")
+    _habanero.Crossref = object
+    sys.modules["habanero"] = _habanero
+
+import conjurer_librarian as lib  # noqa: E402
+from durable_queue import DiskQueue  # noqa: E402
+
+_LOG = logging.getLogger("test-outbox")
+_LOG.addHandler(logging.NullHandler())
+
+
+class _Resp:
+    def __init__(self, status_code, text=""):
+        self.status_code = status_code
+        self.text = text
+
+
+@pytest.fixture(autouse=True)
+def _no_backoff(monkeypatch):
+    # Never actually sleep during retry backoff in tests.
+    monkeypatch.setattr(lib.time, "sleep", lambda _s: None)
+
+
+@pytest.fixture
+def outbox(tmp_path, monkeypatch):
+    box = DiskQueue(str(tmp_path / "outbox"))
+    monkeypatch.setattr(lib, "_outbox", box)
+    return box
+
+
+def test_deliver_succeeds_first_try(monkeypatch):
+    calls = []
+    monkeypatch.setattr(lib.requests, "post", lambda *a, **k: calls.append(1) or _Resp(200))
+    assert lib._deliver_result("u1", {"u1": {}}, _LOG, attempts=3) is True
+    assert len(calls) == 1  # no needless retries after a 200
+
+
+def test_deliver_retries_then_succeeds(monkeypatch):
+    responses = iter([_Resp(503), _Resp(500), _Resp(200)])
+    monkeypatch.setattr(lib.requests, "post", lambda *a, **k: next(responses))
+    assert lib._deliver_result("u2", {"u2": {}}, _LOG, attempts=3) is True
+
+
+def test_deliver_returns_false_when_all_attempts_fail(monkeypatch):
+    def boom(*_a, **_k):
+        raise lib.requests.exceptions.RequestException("bot down")
+
+    monkeypatch.setattr(lib.requests, "post", boom)
+    assert lib._deliver_result("u3", {"u3": {}}, _LOG, attempts=2) is False
+
+
+def test_resend_once_removes_only_acked_entries(outbox, monkeypatch):
+    outbox.put("ok", {"ok": {}})
+    outbox.put("bad", {"bad": {}})
+
+    def fake_deliver(query_uuid, _payload, _logger, attempts=1):
+        return query_uuid == "ok"
+
+    monkeypatch.setattr(lib, "_deliver_result", fake_deliver)
+    lib._resend_once(_LOG)
+
+    assert not outbox.contains("ok")   # acked -> dropped
+    assert outbox.contains("bad")      # not acked -> kept for the next sweep
+
+
+def test_resend_keeps_result_until_bot_recovers(outbox, monkeypatch):
+    # Simulate: bot down for the first sweep, up for the second. The result must
+    # survive the outage and be delivered on recovery.
+    outbox.put("u9", {"u9": {"10.1/x": {"Title": ["P"], "type": "article"}}})
+    state = {"up": False}
+
+    def flaky_post(*_a, **_k):
+        return _Resp(200) if state["up"] else _Resp(502)
+
+    monkeypatch.setattr(lib.requests, "post", flaky_post)
+
+    lib._resend_once(_LOG)               # bot down
+    assert outbox.contains("u9")         # preserved, not lost
+
+    state["up"] = True
+    lib._resend_once(_LOG)               # bot recovered
+    assert not outbox.contains("u9")     # now delivered and cleared

tests/integration/test_result_durable_delivery.py

@@ -0,0 +1,92 @@
+"""Integration: the bot's DURABLE, idempotent result intake (/conjurer).
+
+The expensive-result guarantees on the bot side:
+* every result is persisted to the inbox before it is acked,
+* a resend of a not-yet-delivered result is dropped (no double render),
+* once rendered (mark_delivered) further resends are dropped and it leaves the
+  inbox,
+* on startup, an accepted-but-unrendered result is replayed from the inbox,
+* health-check pongs are never persisted.
+"""
+import pytest
+
+import communication_subroutine as cs
+from durable_queue import DiskQueue
+
+
+@pytest.fixture
+def spool(tmp_path, monkeypatch):
+    inbox = DiskQueue(str(tmp_path / "inbox"))
+    delivered = DiskQueue(str(tmp_path / "delivered"))
+    monkeypatch.setattr(cs, "_inbox", inbox)
+    monkeypatch.setattr(cs, "_delivered", delivered)
+    cs.API_KEY = None
+    while not cs.incoming_q.empty():
+        cs.incoming_q.get()
+    return inbox, delivered
+
+
+def _drain_incoming():
+    out = []
+    while not cs.incoming_q.empty():
+        out.append(cs.incoming_q.get())
+    return out
+
+
+def _payload(uuid):
+    return {uuid: {"10.1000/xyz": {"Title": ["A Real Paper"], "type": "journal-article"}}}
+
+
+def test_result_is_persisted_then_queued(spool):
+    inbox, _delivered = spool
+    resp = cs.app.test_client().post("/conjurer", json=_payload("u1"))
+    assert resp.status_code == 200
+    assert inbox.contains("u1")  # durable before ack
+    assert _drain_incoming() == [_payload("u1")]
+
+
+def test_resend_while_pending_is_not_requeued(spool):
+    client = cs.app.test_client()
+    client.post("/conjurer", json=_payload("u2"))
+    _drain_incoming()  # consume the first queueing
+    # Resend before it was rendered: inbox still holds it -> dropped, not doubled.
+    client.post("/conjurer", json=_payload("u2"))
+    assert _drain_incoming() == []
+
+
+def test_resend_after_delivery_is_dropped(spool):
+    inbox, delivered = spool
+    client = cs.app.test_client()
+    client.post("/conjurer", json=_payload("u3"))
+    _drain_incoming()
+    cs.mark_delivered("u3")
+    assert not inbox.contains("u3")
+    assert delivered.contains("u3")
+    # A late resend of an already-delivered result must not re-render.
+    client.post("/conjurer", json=_payload("u3"))
+    assert _drain_incoming() == []
+
+
+def test_replay_requeues_only_undelivered(spool):
+    inbox, delivered = spool
+    inbox.put("u4", _payload("u4"))
+    inbox.put("u5", _payload("u5"))
+    delivered.put("u5", {})  # u5 already shown to the user
+    cs.replay_inbox()
+    keys = [list(p.keys())[0] for p in _drain_incoming()]
+    assert keys == ["u4"]  # only the un-rendered one replayed
+    assert not inbox.contains("u5")  # the delivered one is cleaned from the inbox
+
+
+def test_empty_result_still_persisted_and_delivered(spool):
+    inbox, _delivered = spool
+    cs.app.test_client().post("/conjurer", json={"u6": {}})
+    assert inbox.contains("u6")
+    assert _drain_incoming() == [{"u6": {}}]
+
+
+def test_pong_is_not_persisted(spool):
+    inbox, _delivered = spool
+    cs.app.test_client().post("/conjurer", json={"__pong__": "ping-1"})
+    assert len(inbox) == 0
+    assert _drain_incoming() == [{"__pong__": "ping-1"}]

tests/unit/test_durable_queue.py

@@ -0,0 +1,76 @@
+"""Unit tests for the disk-backed durable queue used by result delivery."""
+import json
+
+from durable_queue import DiskQueue
+
+
+def test_put_contains_remove(tmp_path):
+    q = DiskQueue(str(tmp_path / "q"))
+    assert not q.contains("a")
+    q.put("a", {"hello": 1})
+    assert q.contains("a")
+    q.remove("a")
+    assert not q.contains("a")
+    q.remove("a")  # idempotent - no error on missing
+
+
+def test_put_overwrites_and_roundtrips_payload(tmp_path):
+    q = DiskQueue(str(tmp_path / "q"))
+    q.put("uuid-1", {"uuid-1": {"10.1/x": {"Title": ["P"], "type": "article"}}})
+    q.put("uuid-1", {"uuid-1": {"changed": True}})
+    items = q.items()
+    assert len(items) == 1
+    key, payload, _ts = items[0]
+    assert key == "uuid-1"
+    assert payload == {"uuid-1": {"changed": True}}
+
+
+def test_items_sorted_oldest_first(tmp_path, monkeypatch):
+    q = DiskQueue(str(tmp_path / "q"))
+    import durable_queue
+
+    times = iter([100.0, 200.0, 300.0])
+    monkeypatch.setattr(durable_queue.time, "time", lambda: next(times))
+    q.put("c", {})
+    q.put("a", {})
+    q.put("b", {})
+    assert [k for k, _p, _ts in q.items()] == ["c", "a", "b"]
+
+
+def test_corrupt_file_is_skipped_not_fatal(tmp_path):
+    directory = tmp_path / "q"
+    q = DiskQueue(str(directory))
+    q.put("good", {"ok": 1})
+    (directory / "broken.json").write_text("{ this is not json", encoding="utf-8")
+    keys = q.keys()
+    assert keys == ["good"]  # broken file skipped, good one survives
+
+
+def test_prune_keeps_newest(tmp_path, monkeypatch):
+    q = DiskQueue(str(tmp_path / "q"))
+    import durable_queue
+
+    times = iter([1.0, 2.0, 3.0, 4.0, 5.0])
+    monkeypatch.setattr(durable_queue.time, "time", lambda: next(times))
+    for key in ("k1", "k2", "k3", "k4", "k5"):
+        q.put(key, {})
+    dropped = q.prune(2)
+    assert dropped == 3
+    assert set(q.keys()) == {"k4", "k5"}
+
+
+def test_atomic_write_leaves_no_tmp_files(tmp_path):
+    directory = tmp_path / "q"
+    q = DiskQueue(str(directory))
+    q.put("a", {"x": 1})
+    leftover = [p.name for p in directory.iterdir() if p.suffix == ".tmp"]
+    assert leftover == []
+
+
+def test_key_with_slashes_is_sanitised(tmp_path):
+    q = DiskQueue(str(tmp_path / "q"))
+    q.put("../../etc/passwd", {"evil": 1})
+    # Stays inside the directory (no traversal), and round-trips by key.
+    files = list((tmp_path / "q").iterdir())
+    assert all(f.parent == tmp_path / "q" for f in files)
+    assert q.items()[0][1] == {"evil": 1}