[sharpca]

Summary

• contract:mail-source had no working local-files backend: the mbox adapter
  documented three read operations and shipped no executable. tools/maildir/
  — already named as the vendor home for this backend in its own README, and
  already carrying the outbound half — now implements them:
  maildir-source-threads, maildir-source-read, maildir-source-thread-url.
• One reader, three shapes sniffed per call: a single mbox file, a Maildir, or
  a directory of .eml files. Read-only by construction; there is no draft or
  send path here, that is maildir-draft-create's half of the vendor.
• Docs follow: tools/maildir/README.md goes from "reader remains the
  documented stub" to an Operations table, and the mbox adapter's own README
  drops its stub status and points at the implementation.

Type of change

• Python package (tools/*/ with pyproject.toml)
• Tool / bridge contract (tools/<system>/*.md)
• Documentation (docs/, README.md, CONTRIBUTING.md)

Test plan

• pytest tests/test_read_archive.py: 16 passed — covers all three archive
  shapes, threading by In-Reply-To and by References, the
  orphan-becomes-its-own-root case, whole-thread retention across the
  --since-days boundary, newest-first ordering, oldest-first thread
  bodies, the missing-thread exit code, and both thread_url branches.
• mypy: no issues in 8 source files
• ruff check and ruff format --check: clean
• markdownlint-cli2 on both changed READMEs: 0 issues

RFC-AI-0004 compliance

• Write-access discipline — the new code is read-only. It opens the
  archive, reads it, and returns JSON; nothing writes, sends, or drafts.

Linked issues

Closes #304

Notes for reviewers

• Two judgement calls are in the commit body rather than repeated here: a
  thread is returned whole when any message is inside the window, and
  thread_url returns a file:// link rather than inventing a public address
  when the adopter declared no template.
• tools/maildir's existing tests fail on Windows because Maildir names carry
  :2,D, which Windows rejects. That is a platform limit, unrelated to this
  change, and CI runs on Linux.
• The mbox row of the capability matrix in tools/mail-source/contract.md
  already claimed these three operations; it now has an implementation behind
  the claim.

Generative AI disclosure

This change was authored with an AI coding agent (WorkBuddy AI, DeepSeek-V4.1-Flash)
under human review. The commit carries the Generated-by: trailer; the
contributor has reviewed the diff and owns the change.

[Kaap10]

Thanks for working on this @sharpca! Landing the read-side for contract:mail-source closes a key gap for forensics/air-gapped triage (#304). The format sniffing, whole-thread retention across --since-days, and orphan-root handling are well-designed.

A few items to address before merge:

1. Rebase needed (Commit hygiene): The branch is based on an older commit and bundles two commits (9f1da7d9 and f432930e) that were already merged via PR #1752 and PR #1753. This makes the PR show 20 files changed across other packages. Please rebase onto main to keep only the single commit for #304:

   git fetch upstream main
   git rebase --onto upstream/main HEAD~1
   git push --force-with-lease origin feat/maildir-archive-read

2. thread_url file URI: In read_archive.py:267, f"file://{root.as_posix()}#{wanted}" produces file://C:/... on Windows (which is missing the third slash for drive letters). Consider using f"{root.as_uri()}#{wanted}" from pathlib.Path for cross-platform RFC-compliant file URIs.

3. CLI FileNotFoundError handling: In threads_main and read_main, catch FileNotFoundError and print to stderr with exit code 1 so missing archive paths exit cleanly rather than dumping an unhandled Python traceback.

4. .eml parsing: In iter_messages (line 156), consider email.message_from_bytes(item.read_bytes()) instead of mailbox.mboxMessage(...) since standalone .eml files lack mbox From envelope delimiters.

Once rebased and updated, this should be ready to land!

[potiuk]

Thanks for landing the read side — the overall shape fits the mail-source contract well. I probed read_archive.py against small synthetic mbox/Maildir archives, and a few things need fixing before this can merge; details are in the inline comments. Most are cheap to cover with tests. (Kaap10's rebase point still stands; that also takes care of the tracker-stats files, which are already on main via #1752.)

One point has no line in this diff: tools/spec-loop/specs/adapters.md (:46–52) still describes tools/maildir/ as drafting-only and points at a separate tools/mail-archive/ reader for mbox/Maildir. AGENTS.md asks for the spec update in this PR or a companion sync-specs PR.

[potiuk]

Inline versions of the points in my review above, on the lines they refer to. The spec point (tools/spec-loop/specs/adapters.md still describes tools/maildir/ as drafting-only) has no line in this diff, so it stays in the review body.

[potiuk]

@sharpca, a couple of things before this can go further:

1. Please split out the two unrelated commits. Besides the maildir read side, this branch carries 9f1da7d (retry 5xx in the tracker-stats dashboard, closing tracker-stats-dashboard: default cache path and transient failures under the secure setup #1744) and f432930 (STALE-CANDIDATE in backlog-stats). Each is a separate change with its own reviewers, so please open them as their own PRs and rebase this one onto main with only the maildir commit. That also keeps the review here focused on the maildir code.
2. The review threads from 2026-10-25 are still open, including the two blocking ones: thread IDs must match the IMAP backend's bracketed form, and replies to a root missing from the archive must stay in one thread. Please push fixes or reply on each thread, and I'll take another look once the head moves.

---

This comment was drafted by an AI-assisted tool and confirmed by an Apache Magpie maintainer.

[sharpca]

Rebased onto main — the branch is now the single maildir commit (7e609f0).
9f1da7d and f432930 are gone from it; they landed via #1752 / #1753.

Spec updated in this PR rather than in a companion one:
tools/spec-loop/specs/adapters.md no longer describes tools/maildir/ as
drafting-only, and tools/mail-archive/ is re-described as the public-archive
interface it is (contract:mail-archive), not a local mbox reader.

Every review thread has a reply. Two things beyond the findings themselves:

1. The thread-id format is pinned, not just fixed: tools/mail-source/contract.md
   § Thread identity now states that every backend declaring
   rfc5322-message-id emits the bracketed <root@host> form, gives the root
   rule (first References id even when absent, else first In-Reply-To, else
   the message's own id), and covers the content-hash kind. The bare form
   would also have been rejected downstream — issue_import/dedup.py only
   accepts <id@host> — so the pin is worth having as a regression guard.
2. Two edges the first cut missed, both now tested: a comment in
   In-Reply-To ((was: report) <r@h>, which RFC 5322 allows) was being
   bracketed into a thread root that never existed, splitting the reply off;
   and the urn:sha256: id emitted for a message with no Message-ID did not
   round-trip through read_thread, so a sweep could list the report and then
   fail to open it.

One difference I left alone: imap_source.thread_root_of stops at
References, so a reply carrying only In-Reply-To is its own thread on IMAP
and joins its parent here. Closing it needs thread_root_of and a
read_thread relaxation (it currently refuses a root outside the fetched
window), so it wants its own PR — happy to open it if you want the two
backends identical in that case too.

Verification: uv run --project tools/maildir --group dev pytest — 33 tests in
tests/test_read_archive.py; ruff check, ruff format --check and mypy
clean on the member.

[potiuk]

Thanks for splitting the branch and for working through every thread: the bracketed thread ids, keying replies on an absent root, the comment in In-Reply-To, the urn:sha256: round trip and the pinned Thread identity section in the contract are all in. Reading the archive as the hostile input it is turned up four more things, so I pushed them as a fixup commit (1902210) rather than send it back:

• HTML stripping was quadratic. The tag patterns could rescan to the end of the body from every <, so a few megabytes of < or of unclosed <script> stalled read_thread for minutes. Every tag pattern now stops at the next <, and a script / style end tag is searched for once, forward; an unclosed element runs to the end of the document.
• A reused Message-ID could hide a report. The second message with an id already seen was dropped, whichever copy the listing yielded first, so any sender could suppress a real report by reusing its id. Copies are now matched on the id and the body, so a report exported twice still counts once and a different message is kept.
• Symlinks were followed. A symlinked .eml, or a symlink in a Maildir's cur/ or new/, would put a local file's bytes into the agent's context. The Maildir is now listed by hand and neither shape follows a symlink.
• IMAP parity. The docstring, the README and the contract said imap_source.searching.thread_root_of applies the same root rule; it does not take the In-Reply-To step yet, and all three now say so. The follow-up you offered for IMAP is welcome.

The tools/spec-loop/.last-sync bump is reverted to main's value, since only adapters.md was synced. New tests cover each case, and the reused-id, symlink and unclosed-script tests fail on the previous head.

Please pull before pushing again.

---

This review was drafted by an AI-assisted tool and
confirmed by an Apache Magpie maintainer. The maintainer
approving this PR has read the findings and signed off. If
something feels off, please reply on the PR and a maintainer
will follow up.

More on how Apache Magpie handles maintainer review:
Contributing guide.

[potiuk]

Thanks again for this one. I filed the IMAP side as #1919: thread_root_of taking the In-Reply-To step, and read_thread returning the reachable messages when the root is outside the fetched window. If you still want to take it, as you offered, it's yours.

[sharpca]

Thanks! I'd be happy to take it.