docs: document the imap4flags implementation

Records why persistence needed no new plumbing (unlike editheader),
the two bugs CTest caught before deployment, and the retrospective
across all four 2026-08-0x Sieve updates this session.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Mario Fetka
2026-08-02 02:29:24 +02:00
parent 16e928d40c
commit bb6f7cbcbb
2 changed files with 101 additions and 0 deletions
+18
View File
@@ -117,6 +117,24 @@ Kept statically linked rather than a loadable module: `address` and
no different in that respect from the other core actions kept out of
`BONGO_SIEVE_MODULE_DIR`.
`imap4flags` (RFC 5232, `setflag`/`addflag`/`removeflag`/`hasflag`) is
another Bongo-authored loadable module (`bongoimap4flags`), like
`editheader` -- Mailutils has no code for this extension at all. Unlike
`editheader`, persisting the result needed no new plumbing: the Store's
`WRITE` protocol has carried a `Z<flags>` token since before this module
existed, and the queue's mailbox-delivery directive line has always had
a trailing message-flags field -- both were simply never populated with
anything but 0 by any caller until now. `setflag`/`addflag`/`removeflag`
read and write a per-evaluation flags accumulator directly (not the
queued-action mechanism `editheader` uses), so a `hasflag` test
correctly sees the effect of an earlier `setflag`/`addflag` within the
same running script. Only the five RFC 5232 base system flags Bongo's
Store itself supports are recognized (`\Answered`, `\Flagged`,
`\Deleted`, `\Seen`, `\Draft` -- no IMAP keywords), and only the single,
unnamed internal flag list -- independent named flag variables and the
`:flags` tag on `fileinto`/`keep` are not implemented, a disclosed
limitation rather than a silent approximation.
Actions include keep, discard, file into a
folder, redirect, reject, and vacation response. Vacation history is durable
so repeated delivery does not generate an immediate reply loop. Scripts and
+83
View File
@@ -5542,6 +5542,89 @@ correctly fell through to the `else` branch. CTest 109/109. Deployed
live, `bongo.service` active, CAPABILITY confirmed advertising
`subaddress`.
**2026-08-02 update**: implemented `imap4flags` (RFC 5232, commit
`16e928d4`), completing the originally identified Sieve RFC gap list.
As with `editheader`, this is a Bongo-authored loadable module
(`bongoimap4flags`, since Mailutils has no code for this extension at
all), but unlike `editheader` it needed no new persistence plumbing: a
`Z<flags>` token on the Store's `WRITE` command already applied straight
to `newdocument.flags` (`StoreCommandWRITE()` in
`src/agents/store/command.c`), and the queue's mailbox-delivery
directive line format already had a trailing message-flags field
(`BongoQueueParseMailboxRecipient()`'s `message_flags` out-param) --
both existed already and were simply never populated with anything but
0: `DeliverToStore()` in `queue.c` took a `messageFlags` parameter
explicitly marked `UNUSED_PARAMETER`, and `rules.c` always sent a
hardcoded `"0"` in the directive line it builds for `fileinto`. Fixed
both to actually carry `setflag`/`addflag`/`removeflag`'s computed
value, confirmed by first tracing the whole chain from Sieve action
through to the Store's own document-flags field before writing any new
code, given the very real risk that the `messageFlags` variable might
have collided with the entirely separate `MSG_FLAG_*` bitmask namespace
(spam-checked, encoding hints, SMTPUTF8, ...) `nmap.h` also defines with
overlapping bit positions -- verified the two are properly distinct
variables in `queue.c`'s `Q_DELIVER` case before touching anything.
`setflag`/`addflag`/`removeflag` read and write a per-evaluation flags
accumulator directly, through two new synchronous public functions
(`BongoSieveGetFlags()`/`BongoSieveSetFlags()`) rather than the queued
`BongoSieveReportAction()` mechanism `editheader` uses -- necessary so a
`hasflag` test correctly observes the effect of an earlier
`setflag`/`addflag` within the same running script, which a deferred,
post-execution accounting pass (as originally sketched, modeled on
`editheader`'s `BONGO_SIEVE_HEADER_EDITED` marker) could not provide.
`BongoSieveEvaluate()` gained a `final_flags` out-parameter for the
implicit-keep case specifically (no explicit `keep`/`fileinto`/
`redirect` in the script), which runs in `rules.c` after the
`mu_sieve_machine_t` -- and so `BongoSieveGetFlags()` -- has already
been destroyed; an earlier attempt routed this through one more queued
action (`BONGO_SIEVE_FLAGS_SNAPSHOT`, reported unconditionally as the
last action of every evaluation) but this broke every existing
consumer's `action.count` expectations by always adding one extra
callback invocation, caught immediately by the native CTest suite and
replaced with the out-parameter before ever being deployed.
Two more bugs, both caught by CTest before deployment: (1)
`require ["imap4flags"];` initially failed ("cannot require
imap4flags") for the same reason `subaddress` did earlier -- needed a
dummy action registered under that exact name, since `require.c`
re-checks the registry for the literal name after a successful dlopen
rather than trusting the load alone. (2) `flags_from_arg()` segfaulted
reading a bare-string argument (`hasflag "\Seen"`, not
`hasflag ["\Seen"]`) -- assumed the required-argument type declaration
(`SVT_STRING_LIST`) meant a bare string always arrived pre-coerced into
a list, and manually inspected `mu_sieve_value_t`'s union on that
assumption; a debugger backtrace showed `val->v.string` was `0x1`, not
a valid pointer. Fixed by using Mailutils' own `mu_sieve_vlist_do()`
instead, the same generic bare-string/list handling
`sieve_test_header()`/`sieve_test_address()` already rely on via the
related `mu_sieve_vlist_compare()` rather than touching the value union
themselves -- the same lesson `subaddress`'s implementation had already
applied by reusing Mailutils' own address/comparator machinery instead
of reimplementing it.
Verified with native CTest cases covering `setflag`/`addflag`/
`removeflag` (including a read-modify-write sequence), `hasflag`'s
default/`:contains` and `:is` semantics, a plain `keep` picking up the
accumulator, and the implicit-keep `final_flags` path with no explicit
action at all -- plus one live delivery test: `setflag
["\Seen","\Flagged"]` before a `fileinto`, confirmed by reading the
delivered document's actual flags field back from the Store afterward
(`0xa`, exactly `STORE_MSG_FLAG_SEEN | STORE_MSG_FLAG_FLAGGED`, with
`\Answered` correctly absent). CTest 109/109. Deployed live,
`bongo.service` active, CAPABILITY confirmed advertising `imap4flags`.
Across SIEVE-04's four 2026-08-0x updates (`editheader`, Tier 1,
`subaddress`, `imap4flags`), the pattern worth naming: every genuinely
new implementation (`editheader`'s persistence, `subaddress`,
`imap4flags`) surfaced at least one bug the native CTest suite caught
before live deployment, and in two cases (the `editheader` fork
incident, the `BONGO_SIEVE_FLAGS_SNAPSHOT` design) an approach was
tried, found wanting, and replaced -- entirely before anything reached
`bongo.service`. The Tier 1 additions (pure allowlist widening, zero
new code) needed no such correction. That difference tracks the
_amount of new code_, not the size of the RFC being implemented.
## SIEVE-05
Result: **PASS**