Files
webmail-client/docs/open-questions.md
T
lukasandClaude Opus 5 149c11581e Antispam C2: learn from what the user does
Explicit teaching shipped with C1 (the Spam / Not spam buttons). This adds the
two halves that need no buttons at all.

Passive learning (§C.5). Moving mail into Spam says "this is spam" more honestly
than any button, because the user is doing it for their own reasons rather than
to train a filter; moving it back out says the opposite. Only those two
transitions teach — Spam to Trash is someone tidying up, Inbox to Archive says
nothing about spam. Teaching the same message the same thing twice is skipped,
since users move things back and forth, but reversing a lesson is allowed. If
the engine is down nothing is recorded, so a later move can teach it again.

This needed the transition, not just the destination: `message.changed` carried
only where a message landed, so `move_job` now also publishes
previous_mailbox_id and both special_use values.

Provider bootstrap (§C.5). An account going live enqueues a job that teaches
Bayes from mail the user already owns — up to 500 messages from the provider's
Junk folder as spam, and up to 500 Inbox messages older than 30 days as ham.
This is the constructive use of the history that classification deliberately
refuses to act on: the same backfill, read instead of judged. It sleeps between
messages so live classification never queues behind training, stops rather than
grinding if the engine disappears, and marks the account done in KV so it runs
once.

That needed `PluginMessages.sample`: the SDK could fetch a message by id but not
enumerate a folder. Ids only — bodies are pulled one at a time, so a 500-message
sample never materialises 500 bodies.

Verified against the running engine: teaching moved its `learned` counter 0 -> 2.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018SqJYt7jD7oUEkoY68sBZo
2026-09-17 08:36:51 +02:00

169 lines
8.9 KiB
Markdown

# Open Questions
Per the setup rules, deviations and unresolved decisions are recorded here rather
than guessed silently.
## OQ-6 — Appendix C needs SDK surfaces the frozen §11 API does not have
**Status:** resolved by additive SDK extension; recorded here because C.8.1 says
"zero core diffs".
**Problem.** C1 cannot be written against the M3 SDK as frozen:
1. **§C.4 step 4** requires moving a message to Junk "through the existing move
machinery, so the verdict propagates to the IMAP server". `PluginActions`
exposed only `set_flags` and `trash`. `trash` moves the *whole thread*, which
is right for Delete and wrong for a spam verdict — one spam reply would drag a
legitimate conversation into Junk. (The `filters` plugin had already noted the
same gap: "a `move` action would follow the same pattern".)
2. **§C.4 step 3** posts the *raw blob* to Rspamd. `PluginMessages` exposed
`envelope()` and `header()` — no way to get the whole RFC822.
3. **§C.4 step 1** processes "only messages landing in `special_use='inbox'`
mailboxes". Neither the `message.new` payload nor `envelope()` said where a
message landed.
**Resolution.** Three additive changes to `app/plugins/`, no change to any core
mail path:
- `PluginActions.move_to_special(user_id, message_id, special_use)` — per-message
move wrapping the same optimistic-update + `move_job` path as the core
endpoint. Returns False (never raises) when the account has no such folder.
- `PluginMessages.raw(user_id, message_id) -> bytes | None`.
- `envelope()` now also returns `account_id` and `mailbox_special_use`.
**C2 needed two more, on the same reasoning:**
4. **§C.5 passive learning** reads the user's own filing as a verdict, which
needs the *transition* — "now in Junk" and "moved out of Junk" are different
facts. `message.changed` carried only the destination, so `move_job` now also
publishes `previous_mailbox_id`, `special_use` and `previous_special_use`.
5. **§C.5 provider bootstrap** samples up to 500 provider-Junk messages and 500
aged Inbox messages. The SDK could fetch a message by id but not enumerate a
folder, so `PluginMessages.sample(account_id, special_use, limit,
older_than_days)` returns ids only — the caller pulls bodies one at a time,
so a 500-message sample never materialises 500 bodies.
**Reading of "zero core diffs":** taken to mean the plugin must not require
changes to core *mail flow or behaviour* — with the plugin absent, mail handling
is byte-identical, which remains true. Extending the plugin SDK is what the SDK
is for, and all three additions are useful beyond antispam. If the intent was
literally "no file under `app/` changes", C1 is not implementable and the spec
needs amending instead.
**Also note:** §C.2 itself mandates compose additions (`rspamd`,
`redis-rspamd`), which are core-repo diffs by any reading. They are placed behind
a compose profile so a deployment that does not want the engine is unaffected.
## OQ-7 — Rspamd cannot be reached through the SDK's guarded HTTP
**Status:** resolved, deliberate.
`sdk.http` applies the §10.6 SSRF guard, which rejects private addresses. That is
correct for user-supplied CalDAV/CardDAV URLs, but Rspamd lives at
`http://rspamd:11333` on the compose network — a private address by definition.
The guard would forbid the only address that works.
The antispam plugin therefore uses `httpx` directly for engine calls. This is
safe for the reason the guard exists: the URL comes from `RSPAMD_URL` in the
operator's environment, never from user input, so there is no request-forgery
surface to protect. User-supplied URLs in this plugin (there are none today)
would still have to go through `sdk.http`.
## OQ-1 — Meilisearch container healthcheck reports `unhealthy` (IPv6 `localhost`)
**Status:** resolved — approved deviation applied. The healthcheck URL in
`docker/docker-compose.yml` was changed from `localhost` to `127.0.0.1`; the
container now reports `healthy`.
**Symptom:** `docker compose ps` shows `meilisearch` as `unhealthy`, even though the
service is fully up and reachable from the host (`curl http://127.0.0.1:7700/health`
returns `{"status":"available"}`).
**Root cause:** the spec's healthcheck is
`wget -qO- http://localhost:7700/health | grep -q available`. Inside the
`getmeili/meilisearch:v1.8` container, `/etc/hosts` maps `localhost` to both
`127.0.0.1` and `::1`, and busybox `wget` tries the IPv6 `::1` first. Meilisearch
listens IPv4-only on `0.0.0.0:7700` (`MEILI_HTTP_ADDR=0.0.0.0:7700`), so the `::1`
connection is refused and the healthcheck fails. `wget http://127.0.0.1:7700/health`
from inside the container succeeds.
**Impact:** cosmetic for direct use (the service works), but real for automation:
`docker compose up --wait` (used by the `reset-db` Make target) and the
"all must be healthy" verification step will hang/fail on this host.
**Proposed minimal fix (one word):** change the healthcheck URL from `localhost`
to `127.0.0.1`:
`test: ["CMD-SHELL", "wget -qO- http://127.0.0.1:7700/health | grep -q available"]`.
This is a deviation from the frozen setup doc, so it is left for explicit approval.
## OQ-5 — Relative BLOB_ROOT breaks across processes with different CWDs
**Status:** resolved in dev by using an absolute path.
`BLOB_ROOT=./var/blobs` (relative) resolves against each process's working
directory. Per the dev-setup doc the API runs from the repo root (`uvicorn
--app-dir backend`) while the worker runs "from backend/", so blobs written by the
worker land in `backend/var/blobs` but the API reads `./var/blobs` at the repo
root — a different, empty directory. This only surfaced once the API began reading
blobs (M1 re-sanitize-on-render / attachments); before that the API only served
sanitized HTML from Postgres.
**Fix:** `.env.dev` now sets an absolute `BLOB_ROOT`. Additionally
`GET /messages/{id}/html` falls back to the stored (possibly stale) sanitized HTML
if a blob is unreadable, so a path/permission issue degrades gracefully instead of
500-ing. `.env.example` keeps the relative default with a note; production should
always use an absolute path or run both processes from the same CWD.
## OQ-4 — Duplicate Message-ID / bulk-sender over-merging (threading)
**Status:** resolved with two refinements (both consistent with §7.1's stated intent).
**Symptom (found while testing):** home.pl's welcome mailer reuses one static
`Message-ID` (and `Date`) for every recipient. Two welcome mails — one per test
account, 104 days apart, byte-identical — collapsed into a single thread.
**Cause & fixes:**
1. **Duplicate Message-ID (§7.1, was only half-implemented).** The indexer now
synthesizes a non-authoritative id when a Message-ID is already present in a
**different account** (a separate delivery). A **same-account** duplicate keeps
the real id, so Gmail-style INBOX + All Mail copies of one message still thread
together. Implemented in `app/indexer/indexer.py::_resolve_message_id`; new
index `ix_messages_user_msgid` (migration 0002).
2. **Subject fallback (§7.3 refinement).** The literal rule ("same subject + within
7 days + shares ≥1 participant") over-merges bulk mail, because the shared
participant can be just the common sender. Subject joining now keys on the
**incoming message's recipients (To/Cc)** overlapping the thread — a genuine
reply is always addressed to someone in the thread, a bulk blast to another
account is not. `app/threading_/threader.py::_find_by_subject`.
Both are minor tightenings of the spec's anti-over-merge intent, not behavior the
spec forbids. Covered by `tests/indexer/test_duplicate_msgid.py` and updated
threading vectors.
## OQ-3 — `__Host-` session cookie vs. plain-HTTP dev
**Status:** resolved with a documented dev fallback.
SPEC §6.1/§10.3 mandate the `__Host-session` cookie prefix, which the browser
only accepts when the cookie is `Secure` **and** sent over HTTPS. The dev setup
(`dev-environment-setup.md`) serves the API over plain HTTP (uvicorn :8000 behind
the Vite HTTP proxy), so a `__Host-`/`Secure` cookie would be silently dropped and
login could never work in dev.
**Decision:** cookie naming/flags are derived per-request from the scheme
(`app/identity/cookies.py`): over HTTPS we emit the spec-mandated
`__Host-session` + `Secure`; over plain HTTP (dev only) we fall back to `session`
without `Secure`. Production (behind TLS) is fully spec-compliant. `SameSite=Lax`,
`HttpOnly`, `Path=/` always. The CSRF cookie follows the same scheme rule but is
readable by JS (double-submit, SPEC §10.3).
## OQ-2 — Repository was not a git repo (resolved)
**Status:** resolved.
The setup doc's conda `activate.d` hook locates `.env.dev` via
`git rev-parse --show-toplevel`, and the overall workflow (commit-per-increment,
per-plugin alembic branches) assumes git. The working directory was not a git repo,
so `.env.dev` did not auto-load on `conda activate`. Ran `git init` (branch `main`);
the hook now loads the env correctly. No spec deviation — the doc already assumes git.