Files
mailcat/reports/script_review_findings.md
T
wienen 4780c18d70 Initial commit
Claude has done a lot of work, but there are still some errors which
cause the mails to be distributed to wrong mail boxes. I am now going to
let codex and claude compete. They will have their own branches and
merge to main as soon as we are somewhere where we can switch AI.
2026-07-04 11:18:05 +02:00

14 KiB

Script review: mailbox creation and redistribution

Date: 2026-07-04

Scope reviewed:

  • maak_mappen.py
  • verplaats_bestaand.py
  • mailrules.sieve
  • upload_sieve.py
  • Supporting scripts: download_mailbox.py, analyse_mailbox.py, review_mailinglists.py, unsubscribe.py, kopieer_naar_backup.py, dagelijks_overzicht.py

Static checks:

  • python3 -m py_compile *.py passes.
  • verplaats_log.json currently contains 14 completed step-1 folders and 3166 processed INBOX UIDs, so at least one real redistribution run has been attempted.

Executive summary

The folder creation script mostly works, but redistribution has several real defects that can explain failed or incomplete moves.

The highest-risk issue is in verplaats_bestaand.py: step 1 opens source folders read-only and then tries to mark messages as deleted. On a normal IMAP server, this makes a true move impossible. The script may copy messages but fail to delete originals, while still logging the folder as completed. That can leave duplicates and prevent a later rerun from correcting the folder.

There is also a folder contract mismatch: maak_mappen.py does not create INBOX.Technisch or INBOX.Technisch.DMARC, while both the Sieve rules and redistribution route mail there. That will fail for those categories unless the folders already exist.

The logs are not strong enough for safe recovery: they record intended work as complete without validating that the source message disappeared and the destination message exists. If the first real run failed partially, the safest next action is to audit actual server folder counts before rerunning with the current log.

Findings

P0: Step 1 selects source folders read-only, then tries to delete messages

File: verplaats_bestaand.py

Relevant lines:

  • Line 284: mail.select(..., readonly=True)
  • Lines 301-303: copy the message and then STORE +FLAGS \Deleted
  • Line 306: EXPUNGE
  • Lines 307-310: reports success and logs the source folder as done

Problem:

Step 1 always selects the source folder with readonly=True, including during real execution. In read-only mode, COPY can succeed, but STORE +FLAGS \Deleted should fail or be ignored by the server. That means this code is not reliably moving messages; it may only copy them.

Impact:

  • Messages may remain in their original folders.
  • Duplicates may be created in destination folders.
  • The script still appends the folder to stap1_klaar, so reruns skip it.
  • This matches the symptom: folder creation works, redistribution appears to fail.

Recommended fix:

  • Select read-only only in dry-run mode:
    • mail.select(..., readonly=dry)
  • Check both COPY and STORE results.
  • Only log a folder as complete if all expected messages were copied and deleted.
  • Prefer UID-based operations in step 1, consistent with step 2.

P0: The redistribution log can mark failed or partial moves as complete

File: verplaats_bestaand.py

Relevant lines:

  • Lines 301-304: STORE result is ignored.
  • Lines 307-310: folder is logged as done regardless of whether gekopieerd == n.
  • Lines 365-369: step 2 logs a UID as processed when COPY succeeds, even though STORE result is not checked inside move_message.
  • Lines 256-268: move_message() returns success based only on COPY.

Problem:

The code treats COPY OK as a successful move. A move is only complete if:

  1. The message was copied to the destination.
  2. The source message was marked deleted.
  3. The deletion was expunged, or a later expunge is guaranteed.

The current code does not verify steps 2 and 3 before writing progress to verplaats_log.json.

Impact:

  • A rerun may skip messages that are still in the source folder.
  • Failures from missing destination folders, read-only source folders, or permission issues can be hidden.
  • verplaats_log.json is currently not a reliable source of truth.

Recommended fix:

  • Make move_message() return false unless both UID COPY and UID STORE +FLAGS \Deleted return OK.
  • Save detailed failures in the log with source, UID, destination, and server response.
  • Do not append to stap1_klaar unless the final moved count equals the expected count.
  • Before rerunning after the current failure, either archive/reset verplaats_log.json or write a repair mode that reconciles the log against actual server state.

P1: maak_mappen.py omits folders targeted by redistribution and Sieve

Files:

  • maak_mappen.py
  • verplaats_bestaand.py
  • mailrules.sieve

Relevant lines:

  • verplaats_bestaand.py line 165 targets Technisch.DMARC.
  • mailrules.sieve lines 230-241 targets Technisch.DMARC and Technisch.
  • maak_mappen.py lines 85-193 contains no Technisch or Technisch.DMARC.

Problem:

The folder creator and routing targets are inconsistent. Static comparison found these Sieve targets missing from maak_mappen.py:

  • Technisch
  • Technisch.DMARC

The dynamic Sieve invoice path Financieel.Facturen.${jaar}.${kwartaal} is not directly comparable, but the concrete year/quarter folders for 2020-2026 are created.

Impact:

  • Technical/DMARC messages fail to move if those folders do not already exist.
  • Sieve fileinto for those folders can fail for new mail.

Recommended fix:

  • Add Technisch and Technisch.DMARC to maak_mappen.py.
  • Add a test or check that every static fileinto destination and every Python route destination exists in the folder creation list.

P1: Step 1 assumes archive parent folders already exist

Files:

  • maak_mappen.py
  • verplaats_bestaand.py

Relevant lines:

  • verplaats_bestaand.py lines 192-198 moves old archive subfolders into INBOX.Archief.2020, INBOX.Archief.2021, and INBOX.Archief.2022.
  • maak_mappen.py lines 188-192 creates only Archief.2023 through Archief.2026.

Problem:

The redistribution script targets INBOX.Archief.2020, INBOX.Archief.2021, and INBOX.Archief.2022, but the folder creator does not create them. In the current offline mailbox they exist, so this may work on the original account. It can fail on a fresh backup account or after recreating the target structure from scratch.

Impact:

  • Archive redistribution may fail on accounts where those parent folders do not already exist.
  • If COPY fails because the destination does not exist, step 1 still risks logging the source as done.

Recommended fix:

  • Include Archief, Archief.2020, Archief.2021, and Archief.2022 in maak_mappen.py, or have verplaats_bestaand.py create missing destination folders before moving.

P1: verplaats_bestaand.py only routes INBOX, despite claiming broader routing

File: verplaats_bestaand.py

Relevant lines:

  • Lines 8-9: docstring says "INBOX + overige mappen".
  • Line 204: comment says folders can be routed per message.
  • Line 205: ROUTE_FOLDERS = ["INBOX"]
  • Lines 315-377: function is hard-coded to INBOX.

Problem:

The documentation implies broader per-message routing, but the implementation only processes INBOX. Existing messages in other folders are only moved if they are covered by the hard-coded whole-folder renames or by the invoice step.

Impact:

  • Many existing messages outside INBOX remain unsorted.
  • The user may interpret this as redistribution failing, when the script never attempted those folders.

Recommended fix:

  • Either make the documentation explicit that only INBOX is routed, or implement routing for a configured set of non-system folders.
  • If broad routing is implemented, exclude destination folders to avoid repeatedly reprocessing already sorted mail.

P1: Routing exact-domain matching is weaker than the reports suggest

File: verplaats_bestaand.py

Relevant lines:

  • Lines 226-228 extracts the exact sender domain.
  • Lines 244-253 uses direct lookup only: DOMAIN_LOOKUP.get(domain).
  • Lines 75-166 contains hand-curated domains, including some but not all subdomains.

Problem:

The redistribution routes only exact domains. If mail comes from mail.example.com, but the route table contains only example.com, it will not match. Some subdomains are listed manually, but this is incomplete by design.

Impact:

  • Valid messages can remain in INBOX as "Geen match".
  • This can look like the redistribution failed even though the script ran.

Recommended fix:

  • Add a parent-domain fallback, for example try a.b.example.com, then b.example.com, then example.com.
  • Keep exceptions for multi-tenant senders where parent-domain matching is unsafe.
  • Log unmatched high-volume domains for review.

P1: Sieve paths may use a different namespace prefix than created folders

Files:

  • maak_mappen.py
  • mailrules.sieve
  • upload_sieve.py

Relevant lines:

  • maak_mappen.py line 16 uses PREFIX = "INBOX.".
  • mailrules.sieve lines 5-7 notes this ambiguity.
  • upload_sieve.py line 21 uses SIEVE_PREFIX = "".

Problem:

Folder creation creates INBOX.<path>, while the uploaded Sieve file currently files into <path> without INBOX.. On some Dovecot configurations this is correct; on others, Sieve needs INBOX.<path>.

Impact:

  • New incoming mail may not land in the created folders even if existing-mail redistribution is fixed.
  • This affects Sieve/new mail more than verplaats_bestaand.py, which does use INBOX..

Recommended fix:

  • Verify the active server namespace with a test message.
  • If Sieve delivery fails, set SIEVE_PREFIX = "INBOX." before upload.
  • Consider adding a --prefix flag instead of editing a constant.

P2: Step 3 has no per-message progress log

File: verplaats_bestaand.py

Relevant lines:

  • Lines 382-427 handles invoice folders.
  • No log writes are made for step 3.

Problem:

Invoice redistribution can move a large number of messages, but the script does not record which UIDs were successfully processed.

Impact:

  • If the run is interrupted, it starts over for whatever remains.
  • That is mostly safe if moves are truly atomic, but unsafe with the current weak move verification.

Recommended fix:

  • Add the same robust move result logging recommended for step 2.
  • Include source folder and UIDVALIDITY or use Message-ID based reconciliation.

P2: Folder and IMAP LIST parsing is fragile for encoded or unusual names

Files:

  • download_mailbox.py
  • kopieer_naar_backup.py
  • dagelijks_overzicht.py
  • maak_mappen.py

Relevant examples:

  • download_mailbox.py lines 22-30
  • kopieer_naar_backup.py lines 36-41
  • dagelijks_overzicht.py lines 89-94
  • maak_mappen.py lines 216-223

Problem:

Several scripts parse LIST responses with regular expressions and inconsistent modified UTF-7 decoding. maak_mappen.py decodes names, but other scripts mostly do not.

Impact:

  • Non-ASCII folders such as Univé can be mishandled in scripts that list/select folders from server responses.
  • This is less likely to be the main redistribution failure because verplaats_bestaand.py mostly uses hard-coded folder names, but it is a reliability issue across the toolkit.

Recommended fix:

  • Centralize IMAP folder encoding/decoding and LIST parsing in one helper module.
  • Use that helper from all IMAP scripts.

P2: kopieer_naar_backup.py silently ignores append failures

File: kopieer_naar_backup.py

Relevant lines:

  • Lines 168-179 appends a message to the destination.
  • Line 179 silently does pass on failure.
  • Lines 270-274 marks the folder complete after one pass.

Problem:

A folder can be marked complete even if some message appends failed.

Impact:

  • Backup completeness cannot be trusted from backup_log.json alone.
  • This matters if the backup account is used as a safety net before live redistribution.

Recommended fix:

  • Count append failures.
  • Do not mark a folder complete if failures occurred.
  • Log failed message IDs and server responses.

P2: upload_sieve.py has an incomplete ManageSieve parser

File: upload_sieve.py

Relevant lines:

  • Lines 38-39 expects to read capabilities.
  • Lines 104-109 _read_capabilities() is pass.
  • Lines 58-64 uses a non-synchronizing literal {len+}.

Problem:

The ManageSieve client is intentionally minimal. If the server sends multi-line capabilities before OK, or does not support non-synchronizing literals, this can behave unpredictably.

Impact:

  • Upload may fail on stricter servers.
  • If it appears to work on the current server, this is not the likely redistribution failure, because redistribution uses IMAP, not ManageSieve.

Recommended fix:

  • Use a tested ManageSieve library, or implement response parsing properly.
  • At minimum, capture and print full server responses when upload fails.

Likely explanation of the current failure

Given the observed state:

  • Folder creation appears to work.
  • verplaats_log.json says all 14 step-1 folder renames were completed.
  • verplaats_log.json says 3166 INBOX UIDs were processed.
  • Step 1 selected folders read-only and then attempted deletion.

The most likely scenario is:

  1. Destination folders were created.
  2. Step 1 copied messages from old folders into new folders.
  3. Deletion from the source folders failed because the source folder was selected read-only.
  4. The script still wrote those old folders to stap1_klaar.
  5. Rerunning the script now skips those old folders, so the failed move is not repaired.

Step 2 may have moved many INBOX messages, but because its log is UID-only and only checks COPY, its success should also be verified against actual mailbox state.

  1. Do not run verplaats_bestaand.py --uitvoeren again with the current script and current log.
  2. Inspect actual server counts for:
    • Old source folders in stap1_klaar
    • Their intended destination folders
    • INBOX
    • Invoice folders
  3. Archive verplaats_log.json before any repair run.
  4. Fix move semantics:
    • No read-only select during real moves.
    • Check COPY, STORE, and final expunge behavior.
    • Log failures explicitly.
  5. Add missing folders to maak_mappen.py.
  6. Add a dry-run audit mode that reports source and destination counts without modifying mail.
  7. Only then run a repair mode that moves remaining source messages and avoids creating duplicates, preferably by comparing Message-ID in source and destination.

Minimal code changes to prioritize

  1. In verplaats_bestaand.py, change step-1 select from readonly=True to readonly=dry.
  2. In move_message(), require UID STORE to return OK before returning success.
  3. In step 1, use UID-based search/copy/store and log the folder complete only when all messages moved.
  4. In maak_mappen.py, add:
    • Technisch
    • Technisch.DMARC
    • Archief
    • Archief.2020
    • Archief.2021
    • Archief.2022
  5. Add a destination-existence preflight to verplaats_bestaand.py before any real move.