Add mavmaild, the read-only IMAP poller that feeds mail intake (#246) #65

Closed
claude wants to merge 1 commits from overnight/email-poller into overnight/email-extract
Contributor

What

cmd/mavmaild — the daemon that actually reads the mailbox. Every interval: open one mailbox read-only, fetch the UIDs not yet handed over, post each message to core over ingest_mail (added on the parent branch). Core extracts and writes task candidates; this daemon writes nothing to the store and cannot create a reminder.

Also: build-mail in the Makefile, mavmaild in the Dockerfile, a fully commented-out compose service with enable instructions, deploy/imap.password gitignored, and the daemon table in CLAUDE.md.

Why this shape

  • Separate daemon, because of the credential. Extraction has to run in mavend (llama-server is bound to loopback inside it), but the IMAP password must not live there. mavpoll set the precedent with the zenmoney token (#125): the module talking to the third party holds the secret, reads it from a file so it never appears in ps, in docker-compose.yml or in shell history, and core never sees it. There is deliberately no -password flag; a test asserts the flag does not exist.
  • Reading leaves no trace. EXAMINE (not SELECT) and BODY.PEEK[], so nothing gets marked \Seen in his mailbox.
  • Off unless configured, at both ends. No -password-file ⇒ the daemon refuses to start. No email block in core ⇒ the first ingest returns ErrUnknownMethod and the reader disables itself instead of hammering a socket that will keep refusing.
  • Mail is personal. Logs are counts and UIDs only — never a subject, sender or body. The seen-state file is a list of message ids from his mailbox, so it is written 0600 and atomically.
  • State is an optimisation, not correctness. Capture dedupes on normalised text, so a re-read produces no duplicate rows; the state file exists to keep a restart from spending multi-second LLM calls re-extracting the lookback window. A corrupt file is a log line, not a failure.

How verified

make build and make test both exit 0; gofmt -l clean.

cmd/mavmaild/main_test.go runs the daemon against a scripted in-process IMAP server (over net.Pipe) and a fake core: bulk mail is filtered before core is asked, already-seen UIDs are not re-fetched on a second poll, a failed ingest is retried on the next poll, ErrUnknownMethod stops at the first message, the state file is 0600 and survives a restart, the high-water mark collapses contiguous runs, a corrupt state file is non-fatal, and run() rejects both missing configuration and an empty password file.

The live half is BLOCKED by design: no IMAP credential exists on this box, so nothing here has talked to a real server. Setup is written up as executable QA steps on the Vikunja task.

Vikunja #246

## What `cmd/mavmaild` — the daemon that actually reads the mailbox. Every interval: open one mailbox read-only, fetch the UIDs not yet handed over, post each message to core over `ingest_mail` (added on the parent branch). Core extracts and writes task **candidates**; this daemon writes nothing to the store and cannot create a reminder. Also: `build-mail` in the Makefile, `mavmaild` in the Dockerfile, a fully commented-out compose service with enable instructions, `deploy/imap.password` gitignored, and the daemon table in CLAUDE.md. ## Why this shape - **Separate daemon, because of the credential.** Extraction has to run in mavend (llama-server is bound to loopback inside it), but the IMAP password must not live there. mavpoll set the precedent with the zenmoney token (#125): the module talking to the third party holds the secret, reads it from a **file** so it never appears in `ps`, in `docker-compose.yml` or in shell history, and core never sees it. There is deliberately no `-password` flag; a test asserts the flag does not exist. - **Reading leaves no trace.** EXAMINE (not SELECT) and `BODY.PEEK[]`, so nothing gets marked `\Seen` in his mailbox. - **Off unless configured, at both ends.** No `-password-file` ⇒ the daemon refuses to start. No `email` block in core ⇒ the first ingest returns `ErrUnknownMethod` and the reader disables itself instead of hammering a socket that will keep refusing. - **Mail is personal.** Logs are counts and UIDs only — never a subject, sender or body. The seen-state file is a list of message ids from his mailbox, so it is written 0600 and atomically. - **State is an optimisation, not correctness.** Capture dedupes on normalised text, so a re-read produces no duplicate rows; the state file exists to keep a restart from spending multi-second LLM calls re-extracting the lookback window. A corrupt file is a log line, not a failure. ## How verified `make build` and `make test` both exit 0; `gofmt -l` clean. `cmd/mavmaild/main_test.go` runs the daemon against a scripted in-process IMAP server (over `net.Pipe`) and a fake core: bulk mail is filtered before core is asked, already-seen UIDs are not re-fetched on a second poll, a failed ingest is retried on the next poll, `ErrUnknownMethod` stops at the first message, the state file is 0600 and survives a restart, the high-water mark collapses contiguous runs, a corrupt state file is non-fatal, and `run()` rejects both missing configuration and an empty password file. **The live half is BLOCKED by design:** no IMAP credential exists on this box, so nothing here has talked to a real server. Setup is written up as executable QA steps on the Vikunja task. Vikunja #246
claude added 1 commit 2026-08-01 01:13:36 +02:00
The extraction seam landed on the previous branch but nothing fed it. This
adds the daemon that does: every interval it opens one mailbox read-only
(EXAMINE + BODY.PEEK, so reading leaves no \Seen behind), fetches the UIDs
it has not handed over yet, and posts each message to core over
ingest_mail. Core runs the model and writes task candidates; this daemon
writes nothing and cannot create a reminder.

It is a separate daemon because of the credential. mavpoll set the
precedent with the zenmoney token (#125): the module talking to the third
party holds the secret, reads it from a file so it never lands in argv, in
docker-compose.yml or in shell history, and core never sees it. There is
deliberately no -password flag, and a test asserts that.

Off unless configured at both ends: without -password-file the daemon
refuses to start, and if core has no email block the first ingest returns
ErrUnknownMethod, which disables the reader instead of hammering a socket
that will keep refusing. A seen-UID state file (0600, atomic write) keeps a
restart from re-extracting the whole lookback window; correctness does not
depend on it, since capture dedupes on normalised text. Logs are counts and
UIDs — no subject, sender or body.

Verified with an in-process IMAP server and a fake core: bulk mail is
filtered before core is asked, seen UIDs are not re-fetched, a failed
ingest is retried next poll, ErrUnknownMethod stops at the first message,
and state survives a restart. The live half is untested by design — no IMAP
credential exists on this box; setup is written up as QA steps.

Vikunja #246
claude reviewed 2026-08-01 11:35:17 +02:00
claude left a comment
Author
Contributor

The credential split holds end to end. The password is read from a file, trimmed and checked for empty. It is never logged, and never put on a struct that outlives the call. mailIngester is a one-method interface, so the daemon's whole reach into core is visibly IngestMail and nothing else. The seen-state being an optimisation rather than a correctness requirement, with CaptureTask dedupe as the real guard, is the right way round. Atomic rename at 0600 for a file of message ids is the correct paranoia level.

Four findings.

1. The commented compose block mounts dbdata into mavmaild, and the header of that same file says it must not.

#   volumes:
#     - sockets:/run/maven
#     - dbdata:/var/lib/maven
#     - ./deploy/imap.password:/run/secrets/imap.password:ro

Line 4 of docker-compose.yml:

# One image (built once), one container per daemon. Only mavend holds the key
# and the db volume; the modules mount just the shared socket dir + models.

dbdata is the encrypted database volume, mounted read-write. The daemon wants it for one JSON file of UIDs. The whole argument for a separate reader is that a compromise on either side does not reach the other. This hands the mail reader the store volume so it can write mail-seen.json. Give it its own named volume, or point -state at a path under a volume that holds nothing else. The mount is also read-write while the reader needs nothing from mavend's data.

2. One message that never ingests successfully stalls the high-water mark forever, and the state file then grows without bound.

mark only advances high through a contiguous run, and a failed ingest is deliberately not marked. Walk it:

  • UID 1000 fails ingest. Maybe the model returned something unparsable, maybe it was the oversized-literal case from PR 63. The log line is written and the loop continues, which is correct.
  • UIDs 1001 and up ingest fine and go into set. high stays at 999, because 1000 is missing.
  • After -lookback of 72 hours, UID 1000 falls out of the SEARCH SINCE window. It is never fetched again, so it is never marked.
  • high is now pinned at 999 for the life of the mailbox. Every UID above it stays in the explicit set forever, and save writes all of them, sorted, every poll.

A year of mail is a few hundred thousand entries rewritten every 15 minutes. Nothing breaks loudly, which is what makes it worth catching now. The type comment says the high-water mark exists "so the explicit set stays small on a mailbox read in order". A single transient failure removes that property permanently. Advance high past any UID older than the lookback window. A UID that can no longer be searched for can never be read again.

3. RunWith reopens the seam PR 63 closed on purpose, and its comment claims the opposite.

PR 63 wrote this, and the reasoning was the point:

// dial is the connection seam. nil means Dial (implicit TLS); the tests set
// it to an in-process fake. Unexported so no configuration path can point
// the reader at a non-TLS transport.
dial func(addr string, timeout time.Duration) (*Conn, error)

This PR deletes the field and exports the same seam as a parameter:

// RunWith is Run with an explicit connection function ... there is no
// configuration path that reaches this, so no deployment can end up talking
// cleartext IMAP.
func (f FetchSince) RunWith(password string, dial func(...) (*Conn, error)) ([]Message, error)

The old guarantee was structural. An unexported field cannot be set from outside internal/email, so the compiler enforced it. The new one is a claim about the callers that exist today. Any code in the tree can now hand RunWith a plaintext dialer, and it gets the password as the first argument. The only reason for the change is that the test moved to package main. Keep the field unexported and expose the seam through an export_test.go in internal/email. Or let the reader build the fake *Conn through a helper internal/email owns.

4. Nothing is stopping when core refuses, and the compose restart policy turns that into a loop.

pollOnce sets disabled and returns. The check for disabled happens on the next tick, so with the default -interval the daemon sits idle for 15 minutes before exiting. It then exits with status 0. The commented service inherits restart: unless-stopped from *image, and compose restarts a clean exit under that policy. So the sequence is: log in to IMAP, fetch, get refused by core, idle 15 minutes, exit, restart, log in again. Forever, at four IMAP logins an hour against a mailbox that has nothing to give. Gmail and Yandex both rate-limit repeated sessions like this.

The log line says "stopping" and the daemon does not stop. Either exit non-zero, or exit immediately when disabled is set rather than at the next tick, or keep running and do nothing.

Smaller notes.

  • pollOnce never sets Junk on IngestMailReq. It counts junk locally and continues. So the field is always false on the wire, and the junk branch in mailIntake.ingest is unreachable through the only real caller. PR 64's IngestMailReq doc says "Junk means the reader's header filter already classified the message as bulk, core is told rather than asked". The reader does not tell it. Either drop the field or send it and let core count bulk.
  • PR 64's IngestMailResp has Skipped, and this daemon ignores it. Today that is fine because the reader never sends junk. If the previous note is fixed by sending Junk instead, Skipped becomes the signal to mark seen without counting a candidate.
  • -max 25 per poll, each ingest being one serialized llama-server call, is the batch that makes PR 64's single-slot contention visible. Not a defect here, but 25 and -interval 15m should be chosen together with whatever bound lands on the core side.
  • r.disabled is set from inside pollOnce and read from the ticker loop, both on the same goroutine, so there is no race today. Worth a line saying so, because the field reads like it wants to be atomic.
  • The state file lands at /var/lib/maven/mail-seen.json per the compose comment, which is the same directory as the database. Pick a distinct path even after finding 1 is fixed. Nobody should be able to restore one from a backup of the other.
The credential split holds end to end. The password is read from a file, trimmed and checked for empty. It is never logged, and never put on a struct that outlives the call. `mailIngester` is a one-method interface, so the daemon's whole reach into core is visibly `IngestMail` and nothing else. The seen-state being an optimisation rather than a correctness requirement, with `CaptureTask` dedupe as the real guard, is the right way round. Atomic rename at 0600 for a file of message ids is the correct paranoia level. Four findings. **1. The commented compose block mounts `dbdata` into mavmaild, and the header of that same file says it must not.** ```yaml # volumes: # - sockets:/run/maven # - dbdata:/var/lib/maven # - ./deploy/imap.password:/run/secrets/imap.password:ro ``` Line 4 of docker-compose.yml: ``` # One image (built once), one container per daemon. Only mavend holds the key # and the db volume; the modules mount just the shared socket dir + models. ``` `dbdata` is the encrypted database volume, mounted read-write. The daemon wants it for one JSON file of UIDs. The whole argument for a separate reader is that a compromise on either side does not reach the other. This hands the mail reader the store volume so it can write `mail-seen.json`. Give it its own named volume, or point `-state` at a path under a volume that holds nothing else. The mount is also read-write while the reader needs nothing from mavend's data. **2. One message that never ingests successfully stalls the high-water mark forever, and the state file then grows without bound.** `mark` only advances `high` through a contiguous run, and a failed ingest is deliberately not marked. Walk it: - UID 1000 fails ingest. Maybe the model returned something unparsable, maybe it was the oversized-literal case from PR 63. The log line is written and the loop continues, which is correct. - UIDs 1001 and up ingest fine and go into `set`. `high` stays at 999, because 1000 is missing. - After `-lookback` of 72 hours, UID 1000 falls out of the `SEARCH SINCE` window. It is never fetched again, so it is never marked. - `high` is now pinned at 999 for the life of the mailbox. Every UID above it stays in the explicit `set` forever, and `save` writes all of them, sorted, every poll. A year of mail is a few hundred thousand entries rewritten every 15 minutes. Nothing breaks loudly, which is what makes it worth catching now. The type comment says the high-water mark exists "so the explicit set stays small on a mailbox read in order". A single transient failure removes that property permanently. Advance `high` past any UID older than the lookback window. A UID that can no longer be searched for can never be read again. **3. `RunWith` reopens the seam PR 63 closed on purpose, and its comment claims the opposite.** PR 63 wrote this, and the reasoning was the point: ```go // dial is the connection seam. nil means Dial (implicit TLS); the tests set // it to an in-process fake. Unexported so no configuration path can point // the reader at a non-TLS transport. dial func(addr string, timeout time.Duration) (*Conn, error) ``` This PR deletes the field and exports the same seam as a parameter: ```go // RunWith is Run with an explicit connection function ... there is no // configuration path that reaches this, so no deployment can end up talking // cleartext IMAP. func (f FetchSince) RunWith(password string, dial func(...) (*Conn, error)) ([]Message, error) ``` The old guarantee was structural. An unexported field cannot be set from outside `internal/email`, so the compiler enforced it. The new one is a claim about the callers that exist today. Any code in the tree can now hand `RunWith` a plaintext dialer, and it gets the password as the first argument. The only reason for the change is that the test moved to package `main`. Keep the field unexported and expose the seam through an `export_test.go` in `internal/email`. Or let the reader build the fake `*Conn` through a helper `internal/email` owns. **4. Nothing is stopping when core refuses, and the compose restart policy turns that into a loop.** `pollOnce` sets `disabled` and returns. The check for `disabled` happens on the next tick, so with the default `-interval` the daemon sits idle for 15 minutes before exiting. It then exits with status 0. The commented service inherits `restart: unless-stopped` from `*image`, and compose restarts a clean exit under that policy. So the sequence is: log in to IMAP, fetch, get refused by core, idle 15 minutes, exit, restart, log in again. Forever, at four IMAP logins an hour against a mailbox that has nothing to give. Gmail and Yandex both rate-limit repeated sessions like this. The log line says "stopping" and the daemon does not stop. Either exit non-zero, or exit immediately when `disabled` is set rather than at the next tick, or keep running and do nothing. Smaller notes. - `pollOnce` never sets `Junk` on `IngestMailReq`. It counts junk locally and `continue`s. So the field is always false on the wire, and the junk branch in `mailIntake.ingest` is unreachable through the only real caller. PR 64's `IngestMailReq` doc says "Junk means the reader's header filter already classified the message as bulk, core is told rather than asked". The reader does not tell it. Either drop the field or send it and let core count bulk. - PR 64's `IngestMailResp` has `Skipped`, and this daemon ignores it. Today that is fine because the reader never sends junk. If the previous note is fixed by sending `Junk` instead, `Skipped` becomes the signal to mark seen without counting a candidate. - `-max 25` per poll, each ingest being one serialized llama-server call, is the batch that makes PR 64's single-slot contention visible. Not a defect here, but 25 and `-interval 15m` should be chosen together with whatever bound lands on the core side. - `r.disabled` is set from inside `pollOnce` and read from the ticker loop, both on the same goroutine, so there is no race today. Worth a line saying so, because the field reads like it wants to be atomic. - The state file lands at `/var/lib/maven/mail-seen.json` per the compose comment, which is the same directory as the database. Pick a distinct path even after finding 1 is fixed. Nobody should be able to restore one from a backup of the other.
kami closed this pull request 2026-08-01 14:51:50 +02:00
Owner

Landed on master. The stack was one linear chain, so #84 carried every commit from #50 up, and master now contains this branch in full. Merging this PR on its own is an empty diff, so it is closed rather than merged. The review findings for it were fixed in the 2026-08-01 pass and are on master as commits on the stack tip, not on this branch.

Landed on master. The stack was one linear chain, so #84 carried every commit from #50 up, and master now contains this branch in full. Merging this PR on its own is an empty diff, so it is closed rather than merged. The review findings for it were fixed in the 2026-08-01 pass and are on master as commits on the stack tip, not on this branch.

Pull request closed

Sign in to join this conversation.
No Reviewers
No Label
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: kami/Maven#65