Fix three problems found in a project review

.dockerignore

@@ -0,0 +1,54 @@
+# What `COPY . .` in docker/Dockerfile and docker/Dockerfile.worker may not see.
+#
+# Without this file the whole working tree is the build context, and the
+# runtime stage copies /app out of the builder - so anything in the tree is in
+# the image that gets pushed. .gitignore does not help: Docker does not read it,
+# and a file git ignores is exactly the kind a developer has locally (.env).
+
+# Secrets. `.env` holds SECRET_KEY, the key every stored AWS secret is encrypted
+# under. A local build pushed to a registry would otherwise carry it.
+.env
+.env.*
+*.pem
+*.key
+
+# Version control and CI: history can hold what was later removed, and none of
+# it runs in the container.
+.git
+.gitattributes
+.githooks
+.github
+.gitmessage
+.pre-commit-config.yaml
+
+# A host virtualenv. The image installs its own at /app/.venv, and copying a
+# Windows or macOS one over it replaces working interpreters with ones that
+# cannot start.
+.venv
+venv
+**/.venv
+
+# Caches and build output.
+__pycache__
+*.py[cod]
+.pytest_cache
+.mypy_cache
+.ruff_cache
+.coverage
+htmlcov
+site
+
+# Not part of the running service: the documentation site, its theme and
+# config, working notes, the specification, and the tests.
+docs
+overrides
+mkdocs.yml
+research
+SESKit_MVP.md
+tests
+
+# Local overrides and editor files.
+docker-compose.override.yml
+.vscode
+.idea
+.DS_Store

apps/worker/src/seskit_worker/main.py

@@ -7,7 +7,7 @@
 
 from typing import Any
 
-from arq import cron
+from arq import cron, func
 from arq.connections import RedisSettings
 from seskit_core.config import get_settings
 from seskit_core.db import dispose_engine
@@ -53,7 +53,9 @@ class WorkerSettings:
         poll_events,
         poll_inbound,
         recheck_identities,
-        send_email,
+        # Its own try count: the class-wide max_tries below is for jobs that
+        # never ask ARQ to retry them, and this one does.
+        func(send_email, max_tries=get_settings().EMAIL_SEND_MAX_ATTEMPTS),
         deliver_webhook,
     ]
 

apps/worker/src/seskit_worker/sending.py

@@ -12,13 +12,21 @@
 retry only when retrying could help: the status moves to `sending` before the
 call, the message id is written immediately after, and a terminal rejection
 fails the message rather than being attempted twice.
+
+**On retrying.** ARQ retries a job only when it raises :class:`arq.Retry`; any
+other exception is a final failure, whatever ``max_tries`` says. A retryable
+provider error therefore raises ``Retry``, and the message stays ``sending``
+between attempts - ``failed`` is not sendable, so recording it early would make
+the retry skip the very message it was scheduled for. The failure is recorded
+on the last attempt, when nothing is left to wait for.
 """
 
 from __future__ import annotations
 
 from collections.abc import Callable
 from typing import Any
 
+from arq import Retry
 from seskit_core.config import get_settings
 from seskit_core.db import get_session_factory
 from seskit_core.errors import APIError, ErrorType
@@ -41,6 +49,11 @@
 
 logger = get_logger(__name__)
 
+#: Seconds to wait before attempt n+1 is ``n * RETRY_BACKOFF_SECONDS``: long
+#: enough for a throttle or a brief outage to clear, short enough that the
+#: three attempts finish within a few minutes.
+RETRY_BACKOFF_SECONDS = 30
+
 #: How the job turns a provider name into an adapter. Injectable so a test can
 #: substitute one without patching a module attribute.
 ProviderBuilder = Callable[..., EmailProviderProtocol]
@@ -89,7 +102,13 @@ async def send_email(ctx: dict[str, Any], email_id: str) -> str:
     """
     factory = get_session_factory()
     async with factory() as session:
-        return await send_one(session, email_id, job_id=ctx.get("job_id"))
+        return await send_one(
+            session,
+            email_id,
+            job_id=ctx.get("job_id"),
+            attempt=int(ctx.get("job_try", 1)),
+            max_attempts=get_settings().EMAIL_SEND_MAX_ATTEMPTS,
+        )
 
 
 async def send_one(
@@ -98,11 +117,16 @@ async def send_one(
     *,
     job_id: str | None = None,
     build: ProviderBuilder | None = None,
+    attempt: int = 1,
+    max_attempts: int = 1,
 ) -> str:
     """Send one queued message. Returns the resulting status.
 
-    Raises on a retryable failure so ARQ tries again; returns normally on a
-    terminal one, because re-sending a rejected message gets the same rejection.
+    Raises :class:`arq.Retry` on a retryable failure while attempts remain, so
+    ARQ tries again; returns normally on a terminal one, because re-sending a
+    rejected message gets the same rejection. ``attempt`` is ARQ's 1-based
+    ``job_try``. The defaults make a single attempt, which is the safe reading
+    for a caller that does not know about retries.
     """
     settings = get_settings()
     build = build or build_provider
@@ -160,17 +184,26 @@ async def send_one(
         )
         result = await provider.send_email(outbound)
     except APIError as error:
-        record_failure(email, error)
+        retryable = is_retryable(error)
+        will_retry = retryable and attempt < max_attempts
+        if will_retry:
+            # Still `sending`, so the next attempt may pick it up. The reason
+            # is kept for the dashboard, which would otherwise show a message
+            # that has been quietly stuck for minutes.
+            email.last_error = error.message
+        else:
+            record_failure(email, error)
         await session.commit()
         logger.info(
             "send_failed",
             email_id=email_id,
             error_type=error.error_type.value,
-            retryable=is_retryable(error),
+            retryable=retryable,
+            attempt=attempt,
+            will_retry=will_retry,
         )
-        if is_retryable(error):
-            # Back to ARQ, which will try again on its own schedule.
-            raise
+        if will_retry:
+            raise Retry(defer=attempt * RETRY_BACKOFF_SECONDS) from error
         return EmailStatus.FAILED.value
 
     record_sent(

docs/guides/ses-sandbox.md

@@ -37,7 +37,7 @@ once all three are. Then: the kind of mail you send, your website, up to four
 contact addresses for AWS's reply, and AWS's acknowledgement to tick. SESKit
 fills in the use case for the reviewer from what it knows - that bounces and
 complaints come back over SNS and the addresses are
-[suppressed](suppression.md), and that every message carries a
+[suppressed](suppression.md), and that a message to a single recipient carries a
 [one-click unsubscribe](suppression.md#one-click-unsubscribe) header.
 
 The card then shows the wait, and the outcome when you refresh. If AWS asks a

packages/core/src/seskit_core/services/production_access.py

@@ -220,8 +220,9 @@ def describe_use_case(mail_type: MailType, readiness: Readiness) -> str:
         "SESKit, a self-hosted email platform on Amazon SES "
         "(https://github.com/Otitodev/seskit). Recipients are the sender's own users. "
         "Bounce and complaint notifications are delivered by SNS to this installation "
-        "and the addresses are suppressed automatically; every message carries a "
-        f"one-click List-Unsubscribe header. {readiness.delivered_count} message(s) "
+        "and the addresses are suppressed automatically; a message to a single "
+        "recipient carries a one-click List-Unsubscribe header. "
+        f"{readiness.delivered_count} message(s) "
         "have been sent and confirmed delivered through this pipeline in the sandbox."
     )
 

packages/provider-aws-ses/src/seskit_provider_aws_ses/provider.py

@@ -310,9 +310,12 @@ def _needs_raw(message: OutboundEmail) -> bool:
     """Whether SES has to be given the assembled MIME rather than fields.
 
     Simple content is built by SES from what we pass, so anything it has no
-    field for - an attachment, a custom header - cannot survive that path.
+    field for - an attachment, a custom header, the List-Unsubscribe pair that
+    ``build_message`` derives from ``unsubscribe_url`` - cannot survive that
+    path. The last is the easy one to forget: it is not a header the caller
+    passed, so nothing in ``headers`` hints that it exists.
     """
-    return bool(message.attachments or message.headers)
+    return bool(message.attachments or message.headers or message.unsubscribe_url)
 
 
 def _content(message: OutboundEmail) -> dict[str, Any]:

tests/test_dockerfiles.py

@@ -228,3 +228,49 @@ def test_alembic_needs_no_configuration_of_its_own() -> None:
     env = (ROOT / "migrations" / "env.py").read_text("utf-8")
     assert "DATABASE_URL" in env
     assert 'set_main_option("sqlalchemy.url"' in env
+
+
+# ----------------------------------------------------------- .dockerignore ---
+
+DOCKERIGNORE = ROOT / ".dockerignore"
+
+
+def _ignored_patterns() -> set[str]:
+    return {
+        line.strip()
+        for line in DOCKERIGNORE.read_text(encoding="utf-8").splitlines()
+        if line.strip() and not line.lstrip().startswith("#")
+    }
+
+
+@pytest.mark.parametrize("pattern", [".env", ".env.*", ".git", ".venv", "*.pem", "*.key"])
+def test_the_build_context_leaves_out_secrets_history_and_the_host_venv(pattern: str) -> None:
+    """`COPY . .` takes the whole context, and the runtime stage copies /app out
+    of the builder, so whatever is not ignored is in the pushed image.
+
+    `.env` carries SECRET_KEY, which every stored AWS secret is encrypted under.
+    A host `.venv` would replace the image's interpreters with ones from another
+    operating system.
+    """
+    assert pattern in _ignored_patterns()
+
+
+def test_the_build_context_keeps_everything_the_build_reads() -> None:
+    """The opposite failure: an ignore rule that breaks the build.
+
+    The build reads the manifests, the lockfile, the sources, the migrations and
+    the entrypoint, and `pyproject.toml` names `README.md`. A pattern covering
+    any of them would fail the build for a reason that looks unrelated.
+    """
+    needed = {
+        "pyproject.toml",
+        "uv.lock",
+        "README.md",
+        "alembic.ini",
+        "apps",
+        "packages",
+        "migrations",
+        "docker",
+        "scripts",
+    }
+    assert not needed & _ignored_patterns()

tests/test_ses_send.py

@@ -124,6 +124,25 @@ async def test_a_custom_header_also_forces_raw(monkeypatch: pytest.MonkeyPatch)
     assert b"X-Campaign: spring" in client.request["Content"]["Raw"]["Data"]
 
 
+async def test_an_unsubscribe_link_forces_raw_and_reaches_the_wire(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """Simple content has no field for List-Unsubscribe either.
+
+    Only `build_message` turns `unsubscribe_url` into the two headers, so a plain
+    message sent as fields went out with neither: SMTP showed the one-click
+    button in local testing and SES silently did not in production.
+    """
+    client = FakeSESClient()
+    url = "https://mail.example.com/u/token"
+
+    await _provider(monkeypatch, client).send_email(_email(unsubscribe_url=url))
+
+    raw = client.request["Content"]["Raw"]["Data"]
+    assert f"List-Unsubscribe: <{url}>".encode() in raw
+    assert b"List-Unsubscribe-Post: List-Unsubscribe=One-Click" in raw
+
+
 async def test_raw_still_passes_the_destination(monkeypatch: pytest.MonkeyPatch) -> None:
     """Otherwise SES takes recipients from the headers, and a blind copy is
     deliberately not in the headers - it would simply never be delivered.

tests/test_worker_sending.py

@@ -15,6 +15,7 @@
 
 import httpx
 import pytest
+from arq import Retry
 from fakes.queue import FakeQueue
 from httpx import AsyncClient
 from seskit_core.errors import APIError, ErrorType
@@ -178,19 +179,65 @@ async def test_a_terminal_rejection_fails_without_retrying(
     assert email.last_error == "Refused."
 
 
-async def test_a_retryable_failure_is_raised_for_arq(
+async def test_a_retryable_failure_asks_arq_to_retry(
     db_session: AsyncSession,
 ) -> None:
-    """A provider that could not be reached may well be reachable in a minute,
-    so the job goes back to the queue rather than giving up.
+    """A provider that could not be reached may well be reachable in a minute.
+
+    ARQ retries only on `Retry` - any other exception is final - and the message
+    must stay sendable, or the retry would skip the message it was scheduled for.
     """
     provider = StubProvider(error=APIError(ErrorType.PROVIDER_ERROR, "Unreachable."))
     email = await _queued(db_session)
 
-    with pytest.raises(APIError):
-        await sending.send_one(db_session, email.id, build=_builder(provider))
+    with pytest.raises(Retry):
+        await sending.send_one(
+            db_session, email.id, build=_builder(provider), attempt=1, max_attempts=3
+        )
+
+    await db_session.refresh(email)
+    assert email.status == EmailStatus.SENDING.value
+    assert email.is_sendable
+    assert email.last_error == "Unreachable."
+
+
+async def test_a_retry_after_a_transient_failure_sends_the_message(
+    db_session: AsyncSession,
+) -> None:
+    """The regression: the first attempt used to mark the message `failed`, and
+    the second attempt then skipped it as settled."""
+    failing = StubProvider(error=APIError(ErrorType.PROVIDER_ERROR, "Throttled."))
+    email = await _queued(db_session)
+    with pytest.raises(Retry):
+        await sending.send_one(
+            db_session, email.id, build=_builder(failing), attempt=1, max_attempts=3
+        )
+
+    working = StubProvider()
+    status = await sending.send_one(
+        db_session, email.id, build=_builder(working), attempt=2, max_attempts=3
+    )
 
     await db_session.refresh(email)
+    assert status == EmailStatus.SENT.value
+    assert email.status == EmailStatus.SENT.value
+    assert email.last_error is None
+    assert len(working.sent) == 1
+
+
+async def test_the_last_attempt_records_the_failure(
+    db_session: AsyncSession,
+) -> None:
+    provider = StubProvider(error=APIError(ErrorType.PROVIDER_ERROR, "Unreachable."))
+    email = await _queued(db_session)
+
+    status = await sending.send_one(
+        db_session, email.id, build=_builder(provider), attempt=3, max_attempts=3
+    )
+
+    await db_session.refresh(email)
+    assert status == EmailStatus.FAILED.value
+    assert email.status == EmailStatus.FAILED.value
     assert email.last_error == "Unreachable."