M4: Wire FilterEngine into live sync so matched rules actually fire #190

Closed
opened 2026-07-28 14:04:28 +00:00 by KofTwentyTwo · 1 comment
KofTwentyTwo commented 2026-07-28 14:04:28 +00:00 (Migrated from github.com)

Context

Story for milestone M4. Read docs/STORY-WORKFLOW.md first — it defines the process, gates, and patterns; this issue defines only the specific work. nuncio_filter::FilterEngine::evaluate (crates/nuncio-filter/src/engine.rs) is a real, well-tested match-and-act pipeline — but it is never called anywhere in production. Verified by reading the whole call graph: crates/nunciod/src/sync.rs's fetch_and_persist (the only place inbound messages are fetched and persisted) calls only backend.sync_folders()/backend.sync_messages()/db.save_email() — no filter engine in sight. crates/nunciod/src/orchestrator.rs has no filter references at all. The ONLY production call sites of FilterEngine are in crates/nunciod/src/grpc.rs's Filters gRPC service (rule CRUD/validate/preview — management only, never evaluated against a real synced message) and crates/nunciod/src/main.rs (constructs the engine and passes it to grpc::serve). Grepping the whole tree confirms DatabaseEngine::save_pending_mutation and DatabaseEngine::save_filter_execution_log are called ONLY from tests — nothing in production ever writes an outbox mutation or an execution log. There IS already a background outbox worker in main.rs that polls db.list_pending_mutations/update_mutation_status on a 5s tick and applies exponential backoff via nuncio_filter::OutboxManager — but since nothing ever enqueues a mutation, that worker has never had anything real to process. Filters are entirely decorative today: a user can create a rule via filter create, it validates and persists, but a real incoming message matching it never does anything.

Goal

After a real inbound sync fetches and persists a message, evaluate it against the daemon's live FilterEngine and make matching rules' actions actually happen: MARK READ/MARK UNREAD apply immediately to the stored message; MOVE TO/COPY TO/FLAG/UNFLAG/DELETE/FORWARD TO/CALL WEBHOOK enqueue a real PendingRemoteMutation row via the existing outbox (OutboxManager::create_mutation + DatabaseEngine::save_pending_mutation) for the existing background worker to pick up. Every match must append a FilterExecutionLog entry (DatabaseEngine::save_filter_execution_log) so there is an honest, hash-chained audit trail of what fired. This must NOT fabricate success: this story does not need to make the outbox worker actually perform the real IMAP/JMAP mutation or the real webhook HTTP call against a live server (that remains a documented future step, exactly as main.rs's outbox worker comment already states) — but it MUST be honestly true that a real match now produces a real, persisted, observable side effect (an immediate DB mutation, a queued outbox row, and a log entry) where today it produces nothing at all.

Files to touch / patterns to mirror

  • crates/nunciod/src/sync.rs — the function to change is fetch_and_persist (around line 54): after each db.save_email(&email).await?, evaluate the just-saved email against the filter engine and route any matched actions. sync_with_backend/run_account_sync/run_all_accounts_sync (all in this file) need a &FilterEngine (or Arc<FilterEngine>) parameter threaded through, since none of them currently take one. Every existing test in this file's #[cfg(test)] mod tests that calls these functions will need updating to pass a FilterEngine (an empty FilterEngine::new(Vec::new()) for tests that don't care about filtering, matching the pattern already used throughout crates/nunciod/src/grpc.rs's tests, e.g. line 1272 FilterEngine::new(Vec::new()).expect("empty rule set")).
  • crates/nunciod/src/main.rs — filter_engine is already constructed at line 41 (Arc::new(FilterEngine::new(initial_rules)?)) BEFORE the sync command consumer task is spawned (line 57). Clone it into that tokio::spawn closure and pass it through to nunciod::sync::run_all_accounts_sync/run_account_sync at the call sites around lines 61-76.
  • crates/nuncio-filter/src/outbox.rs — OutboxManager::create_mutation(rule_id, message_id, action_type, target) (line 35) already builds a PendingRemoteMutation; use it for every non-immediate RuleAction. action_type should be the action's NSQL keyword (e.g. via RuleAction::to_nsql() in crates/nuncio-filter/src/ast.rs, or a shorter fixed tag like "MOVE"/"DELETE"/"FORWARD"/"WEBHOOK" — pick one and be consistent, since nothing downstream currently parses mutation_type back into a RuleAction). target is the action's parameter (folder name / forward address / webhook URL), None for Flag/Unflag/Delete.
  • crates/nuncio-store/src/db.rs — primitives already exist and need no changes: set_message_read (line 1043, for MarkRead/MarkUnread), save_pending_mutation (line 1149), save_filter_execution_log (line 1240, hash-chained ledger).
  • crates/nuncio-filter/src/engine.rs — FilterEngine::evaluate(&self, email: &Email) -> Vec<(FilterRule, Vec<RuleAction>)> (line 256) is the entry point to call; it already scopes by target_account correctly (do not call evaluate_condition directly, evaluate already gates on matches_account first).
  • crates/nuncio-mail/src/mock.rs — MockMailBackend (no changes needed) is what your new offline E2E will use to synthesize a matching message.

Implementation steps

  1. Design and add a small, reusable helper (suggested home: a new apply_filter_actions function in crates/nunciod/src/sync.rs, or a new crates/nunciod/src/filter_apply.rs module) with a signature like async fn apply_filter_actions(db: &DatabaseEngine, filter_engine: &FilterEngine, email: &nuncio_core::model::Email) -> usize (returns count of actions applied, for tests/logging) that:
    • Calls filter_engine.evaluate(email).
    • For each (rule, actions) match, for each action in actions:
      • RuleAction::MarkRead / MarkUnread → db.set_message_read(&email.id, matches!(action, RuleAction::MarkRead)).await.
      • MoveTo/CopyTo/Flag/Unflag/Delete/ForwardTo/CallWebhook → build a PendingRemoteMutation via OutboxManager::create_mutation(&rule.id, &email.id, <tag>, <target>) and persist it via db.save_pending_mutation.
      • Always call db.save_filter_execution_log(&rule.id, &email.id, <action description>) for the match.
    • Log (via tracing::warn!) and continue on any individual DB write failure — a filter-action bookkeeping failure must never abort the rest of the sync (mirror run_all_accounts_sync's existing per-account error-isolation style), but it also must never be silently reported as success either; make sure the eventual return value / logging is honest about partial failures.
  2. Call this helper from fetch_and_persist immediately after each successful db.save_email(&email).await?, threading filter_engine: &FilterEngine through fetch_and_persist, sync_with_backend, run_account_sync, run_all_accounts_sync.
  3. Update main.rs to clone filter_engine into the sync command consumer task and pass it to both call sites.
  4. Update every existing test call site in crates/nunciod/src/sync.rs's test module to pass a FilterEngine (empty rule set where irrelevant to the test).
  5. Add new tests in crates/nunciod/src/sync.rs proving the headline behaviour:
    • A rule WHERE subject CONTAINS 'Urgent' ACTION MARK READ + a MockMailBackend message with a matching subject and read: false → after sync_with_backend/run_account_sync, db.get_message(...) shows read == true (an immediate action genuinely fired).
    • A rule WHERE subject CONTAINS 'Archive Me' ACTION MOVE TO 'Archive' + a matching message → after sync, db.list_pending_mutations(10) contains a pending PendingRemoteMutation with the right rule_id/message_id/mutation tag/target (a remote action was genuinely queued, not silently dropped).
    • Both cases also produce a FilterExecutionLog row (db.list_filter_execution_logs(10)).
    • A non-matching message produces no mutation/log/DB change (negative case).
  6. Run cargo verify.

Acceptance criteria

  • FilterEngine::evaluate is called for every message persisted by a real (or mock) inbound sync, scoped correctly to the message's account (already guaranteed by evaluate itself).
  • MARK READ/MARK UNREAD matches genuinely flip the stored message's read flag via DatabaseEngine::set_message_read.
  • MOVE TO/COPY TO/FLAG/UNFLAG/DELETE/FORWARD TO/CALL WEBHOOK matches genuinely enqueue a PendingRemoteMutation row (status pending) via the existing outbox primitives — nothing is silently dropped.
  • Every match appends a FilterExecutionLog entry.
  • Offline E2E (using MockMailBackend, no live network) proves a synced message matching a rule actually fires its action end to end (read-flag flip for an immediate action; a real queued outbox row for a remote action) — this is the concrete proof that filters are no longer decorative.
  • A non-matching message produces no side effects (regression coverage).
  • No proto change (no client-visible API surface changes; this is pure engine wiring).
  • cargo verify green (fmt + clippy -D warnings all-targets + test-all), 0 ignored.

Definition of done & process

Follow docs/STORY-WORKFLOW.md: no fabricated data; comment convention (intent-only, no story/GH/phase refs in .rs/.proto); mock-only tests. Branch story/m4-wire-filter-engine off dev; PR to dev with the gate output pasted as evidence; the PO reviews before the issue is closed.

Dependencies / notes

No proto coordination needed. This is the headline "filters are decorative today" fix and the other three M4 stories build on it: factor the evaluate-and-route helper (step 1) as a standalone, reusable function (not inlined into fetch_and_persist) — M4-S4 (bulk triage over gRPC) needs to call the exact same evaluate-and-route logic against a keyset-chunked scan of the whole store, and duplicating it there instead of reusing this story's helper would let the two implementations drift. If M4-S4 starts before this merges, its author should coordinate with you (or the PO) on the helper's exact signature rather than re-deriving it independently. Actually executing the real remote IMAP/JMAP mutation or the real webhook HTTP call against a live server remains explicitly out of scope (the existing outbox worker in main.rs already documents, in its own comment, why it deliberately never marks a mutation "completed" yet) — that is future work once a real mutation-execution transport is wired in. Size: L.

## Context Story for milestone M4. **Read [`docs/STORY-WORKFLOW.md`](https://github.com/KofTwentyTwo/nuncio/blob/dev/docs/STORY-WORKFLOW.md) first** — it defines the process, gates, and patterns; this issue defines only the specific work. `nuncio_filter::FilterEngine::evaluate` (`crates/nuncio-filter/src/engine.rs`) is a real, well-tested match-and-act pipeline — but it is **never called anywhere in production**. Verified by reading the whole call graph: `crates/nunciod/src/sync.rs`'s `fetch_and_persist` (the only place inbound messages are fetched and persisted) calls only `backend.sync_folders()`/`backend.sync_messages()`/`db.save_email()` — no filter engine in sight. `crates/nunciod/src/orchestrator.rs` has no filter references at all. The ONLY production call sites of `FilterEngine` are in `crates/nunciod/src/grpc.rs`'s `Filters` gRPC service (rule CRUD/validate/preview — management only, never evaluated against a real synced message) and `crates/nunciod/src/main.rs` (constructs the engine and passes it to `grpc::serve`). Grepping the whole tree confirms `DatabaseEngine::save_pending_mutation` and `DatabaseEngine::save_filter_execution_log` are called ONLY from tests — nothing in production ever writes an outbox mutation or an execution log. There IS already a background outbox worker in `main.rs` that polls `db.list_pending_mutations`/`update_mutation_status` on a 5s tick and applies exponential backoff via `nuncio_filter::OutboxManager` — but since nothing ever enqueues a mutation, that worker has never had anything real to process. **Filters are entirely decorative today**: a user can create a rule via `filter create`, it validates and persists, but a real incoming message matching it never does anything. ## Goal After a real inbound sync fetches and persists a message, evaluate it against the daemon's live `FilterEngine` and make matching rules' actions actually happen: `MARK READ`/`MARK UNREAD` apply immediately to the stored message; `MOVE TO`/`COPY TO`/`FLAG`/`UNFLAG`/`DELETE`/`FORWARD TO`/`CALL WEBHOOK` enqueue a real `PendingRemoteMutation` row via the existing outbox (`OutboxManager::create_mutation` + `DatabaseEngine::save_pending_mutation`) for the existing background worker to pick up. Every match must append a `FilterExecutionLog` entry (`DatabaseEngine::save_filter_execution_log`) so there is an honest, hash-chained audit trail of what fired. This must NOT fabricate success: this story does not need to make the outbox worker actually perform the real IMAP/JMAP mutation or the real webhook HTTP call against a live server (that remains a documented future step, exactly as `main.rs`'s outbox worker comment already states) — but it MUST be honestly true that a real match now produces a real, persisted, observable side effect (an immediate DB mutation, a queued outbox row, and a log entry) where today it produces nothing at all. ## Files to touch / patterns to mirror - `crates/nunciod/src/sync.rs` — the function to change is `fetch_and_persist` (around line 54): after each `db.save_email(&email).await?`, evaluate the just-saved `email` against the filter engine and route any matched actions. `sync_with_backend`/`run_account_sync`/`run_all_accounts_sync` (all in this file) need a `&FilterEngine` (or `Arc<FilterEngine>`) parameter threaded through, since none of them currently take one. Every existing test in this file's `#[cfg(test)] mod tests` that calls these functions will need updating to pass a `FilterEngine` (an empty `FilterEngine::new(Vec::new())` for tests that don't care about filtering, matching the pattern already used throughout `crates/nunciod/src/grpc.rs`'s tests, e.g. line 1272 `FilterEngine::new(Vec::new()).expect("empty rule set")`). - `crates/nunciod/src/main.rs` — `filter_engine` is already constructed at line 41 (`Arc::new(FilterEngine::new(initial_rules)?)`) BEFORE the sync command consumer task is spawned (line 57). Clone it into that `tokio::spawn` closure and pass it through to `nunciod::sync::run_all_accounts_sync`/`run_account_sync` at the call sites around lines 61-76. - `crates/nuncio-filter/src/outbox.rs` — `OutboxManager::create_mutation(rule_id, message_id, action_type, target)` (line 35) already builds a `PendingRemoteMutation`; use it for every non-immediate `RuleAction`. `action_type` should be the action's NSQL keyword (e.g. via `RuleAction::to_nsql()` in `crates/nuncio-filter/src/ast.rs`, or a shorter fixed tag like `"MOVE"`/`"DELETE"`/`"FORWARD"`/`"WEBHOOK"` — pick one and be consistent, since nothing downstream currently parses `mutation_type` back into a `RuleAction`). `target` is the action's parameter (folder name / forward address / webhook URL), `None` for `Flag`/`Unflag`/`Delete`. - `crates/nuncio-store/src/db.rs` — primitives already exist and need no changes: `set_message_read` (line 1043, for `MarkRead`/`MarkUnread`), `save_pending_mutation` (line 1149), `save_filter_execution_log` (line 1240, hash-chained ledger). - `crates/nuncio-filter/src/engine.rs` — `FilterEngine::evaluate(&self, email: &Email) -> Vec<(FilterRule, Vec<RuleAction>)>` (line 256) is the entry point to call; it already scopes by `target_account` correctly (do not call `evaluate_condition` directly, `evaluate` already gates on `matches_account` first). - `crates/nuncio-mail/src/mock.rs` — `MockMailBackend` (no changes needed) is what your new offline E2E will use to synthesize a matching message. ## Implementation steps 1. Design and add a small, reusable helper (suggested home: a new `apply_filter_actions` function in `crates/nunciod/src/sync.rs`, or a new `crates/nunciod/src/filter_apply.rs` module) with a signature like `async fn apply_filter_actions(db: &DatabaseEngine, filter_engine: &FilterEngine, email: &nuncio_core::model::Email) -> usize` (returns count of actions applied, for tests/logging) that: - Calls `filter_engine.evaluate(email)`. - For each `(rule, actions)` match, for each `action` in `actions`: - `RuleAction::MarkRead` / `MarkUnread` → `db.set_message_read(&email.id, matches!(action, RuleAction::MarkRead)).await`. - `MoveTo`/`CopyTo`/`Flag`/`Unflag`/`Delete`/`ForwardTo`/`CallWebhook` → build a `PendingRemoteMutation` via `OutboxManager::create_mutation(&rule.id, &email.id, <tag>, <target>)` and persist it via `db.save_pending_mutation`. - Always call `db.save_filter_execution_log(&rule.id, &email.id, <action description>)` for the match. - Log (via `tracing::warn!`) and continue on any individual DB write failure — a filter-action bookkeeping failure must never abort the rest of the sync (mirror `run_all_accounts_sync`'s existing per-account error-isolation style), but it also must never be silently reported as success either; make sure the eventual return value / logging is honest about partial failures. 2. Call this helper from `fetch_and_persist` immediately after each successful `db.save_email(&email).await?`, threading `filter_engine: &FilterEngine` through `fetch_and_persist`, `sync_with_backend`, `run_account_sync`, `run_all_accounts_sync`. 3. Update `main.rs` to clone `filter_engine` into the sync command consumer task and pass it to both call sites. 4. Update every existing test call site in `crates/nunciod/src/sync.rs`'s test module to pass a `FilterEngine` (empty rule set where irrelevant to the test). 5. Add new tests in `crates/nunciod/src/sync.rs` proving the headline behaviour: - A rule `WHERE subject CONTAINS 'Urgent' ACTION MARK READ` + a `MockMailBackend` message with a matching subject and `read: false` → after `sync_with_backend`/`run_account_sync`, `db.get_message(...)` shows `read == true` (an immediate action genuinely fired). - A rule `WHERE subject CONTAINS 'Archive Me' ACTION MOVE TO 'Archive'` + a matching message → after sync, `db.list_pending_mutations(10)` contains a `pending` `PendingRemoteMutation` with the right `rule_id`/`message_id`/mutation tag/target (a remote action was genuinely queued, not silently dropped). - Both cases also produce a `FilterExecutionLog` row (`db.list_filter_execution_logs(10)`). - A non-matching message produces no mutation/log/DB change (negative case). 6. Run `cargo verify`. ## Acceptance criteria - [ ] `FilterEngine::evaluate` is called for every message persisted by a real (or mock) inbound sync, scoped correctly to the message's account (already guaranteed by `evaluate` itself). - [ ] `MARK READ`/`MARK UNREAD` matches genuinely flip the stored message's read flag via `DatabaseEngine::set_message_read`. - [ ] `MOVE TO`/`COPY TO`/`FLAG`/`UNFLAG`/`DELETE`/`FORWARD TO`/`CALL WEBHOOK` matches genuinely enqueue a `PendingRemoteMutation` row (status `pending`) via the existing outbox primitives — nothing is silently dropped. - [ ] Every match appends a `FilterExecutionLog` entry. - [ ] Offline E2E (using `MockMailBackend`, no live network) proves a synced message matching a rule actually fires its action end to end (read-flag flip for an immediate action; a real queued outbox row for a remote action) — this is the concrete proof that filters are no longer decorative. - [ ] A non-matching message produces no side effects (regression coverage). - [ ] No proto change (no client-visible API surface changes; this is pure engine wiring). - [ ] `cargo verify` green (fmt + clippy `-D warnings` all-targets + test-all), 0 ignored. ## Definition of done & process Follow [`docs/STORY-WORKFLOW.md`](https://github.com/KofTwentyTwo/nuncio/blob/dev/docs/STORY-WORKFLOW.md): no fabricated data; comment convention (intent-only, no story/GH/phase refs in `.rs`/`.proto`); mock-only tests. Branch `story/m4-wire-filter-engine` off `dev`; PR to `dev` with the gate output pasted as evidence; the PO reviews before the issue is closed. ## Dependencies / notes No proto coordination needed. This is the headline "filters are decorative today" fix and the other three M4 stories build on it: **factor the evaluate-and-route helper (step 1) as a standalone, reusable function** (not inlined into `fetch_and_persist`) — `M4-S4` (bulk triage over gRPC) needs to call the exact same evaluate-and-route logic against a keyset-chunked scan of the whole store, and duplicating it there instead of reusing this story's helper would let the two implementations drift. If `M4-S4` starts before this merges, its author should coordinate with you (or the PO) on the helper's exact signature rather than re-deriving it independently. Actually executing the real remote IMAP/JMAP mutation or the real webhook HTTP call against a live server remains explicitly out of scope (the existing outbox worker in `main.rs` already documents, in its own comment, why it deliberately never marks a mutation "completed" yet) — that is future work once a real mutation-execution transport is wired in. Size: L.
KofTwentyTwo commented 2026-07-30 00:03:52 +00:00 (Migrated from github.com)

Delivered via PR #222 (merged to dev). Filters now genuinely fire in the live sync path (immediate MARK actions + real outbox enqueue + FilterExecutionLog). Adversarial review caught a re-fire bug (duplicate outbox rows/ledger entries on non-incremental re-sync) — fixed with a fire-on-new-insert guard (is_new_message before save_email) + regression test. Reusable apply_filter_actions helper in place for #193.

Delivered via PR #222 (merged to dev). Filters now genuinely fire in the live sync path (immediate MARK actions + real outbox enqueue + FilterExecutionLog). Adversarial review caught a re-fire bug (duplicate outbox rows/ledger entries on non-incremental re-sync) — fixed with a fire-on-new-insert guard (is_new_message before save_email) + regression test. Reusable apply_filter_actions helper in place for #193.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: KofTwentyTwo/nuncio#190