feat(engagement): the rules engine, cooldowns and outbox (engagement Phase 4a) #170

wtclaude · 2026-08-29T13:08:33Z

wtclaude commented

2026-08-29 13:08:33 +00:00

Engagement Phase 4, split 4a / 4b at the org lead's direction. Companion docs PR: docs#183.

This is 4a: the engine, server only, with no HTTP surface at all. It is provably done when a fired trigger produces an outbox row and a send-log entry with no UI in the picture. Admin → Engagement → Rules and the §5.1a segment composition UI are 4b.

Three decisions settled before any code

	Question	Decision
§7.1 Q2	multi-instance: `SKIP LOCKED`, or document single-instance?	Neither: a compare-and-set claim into the `sending` state §4.2a's ENUM already carried
§7.1 Q4	where the engagement admin surface lives	its own top-level nav group — built in 4b
—	one PR or two	4a / 4b

Q2's answer makes the outbox safe for two app instances. It does not make the deployment multi-instance — announceWorker, teamDigestWorker, teamForumUploadSweep and teamActivityPrune are all still written for one, and widening them is not this phase's scope. What it buys is that the one table that will carry mail is ready for the day it is.

What lands

Five tables (engagement_rules, engagement_audience_segments, engagement_cooldowns, engagement_outbox, engagement_sends), src/engagement/{engine,conditions,segments,audiences}.js, src/model/engagement/*, and utils/engagementWorker.js wired into server.js beside its five siblings. engagementEmit's Phase 2 log line becomes the engine call.

The gate order is the design, and two placements are load-bearing:

The G24 ceiling is re-checked at SEND time, not only at save. The save path already ran the same ceilings.permits, so the only way this can fail is the case it exists for — a module upgrade that narrows its trigger's declaration underneath a rule saved when it was wider. Without it, a rule written against yesterday's declaration keeps reaching yesterday's population forever. This is the second call site §5.1a promised, not a second implementation.
The hourly ceiling is checked before the cooldown. The ceiling is about the rule; the cooldown is about one recipient. A rule at its ceiling should not also burn everybody's cooldown slot on sends that never happen.

Two defects in ENGAGEMENT.md's own §4, both found by building it

1. §4.2a's global UNIQUE (dedupe_key) was data loss, not a style question. A dedupe key names the event — "house 0x4001 entered IDOC" — and one event legitimately becomes many rows: fifty recipients is fifty rows, a rule spanning email and in-app doubles that, a second rule doubles it again. Under a global unique index the first insert wins and every other one is silently ignored: ninety-nine recipients dropped by the mechanism meant to stop a replay becoming a second mail. Shipped as UNIQUE (rule_id, user_id, channel, dedupe_key).

2. §4.1's single INSERT … ON DUPLICATE KEY UPDATE cooldown claim always passes against this codebase's pool. Its answer is read out of affectedRows — 1 inserted, 2 updated-and-changed, 0 for a duplicate key whose update changed nothing, that 0 being "still cooling". The mariadb connector defaults foundRows: true, which makes affectedRows report rows matched rather than changed, and utils/db.js does not override it. The no-op returns 1, indistinguishable from a fresh insert. Every cooldown passes, always. It is two statements now — a guarded UPDATE with the interval in a WHERE clause, falling back to INSERT IGNORE for a first fire — still race-free in both directions.

The second defect is why this PR has two test files. engagementEngine.test.js stubs the five tables and was green against the broken claim, because a stub can only agree with whoever wrote it and the same misreading produced both. engagementEngineSql.test.js runs the raw statements against a real MariaDB, creates a throwaway database and drops it, and skips when there is none so CI stays green without one. A stub is a fine stand-in for a table and a poor one for a protocol.

Segments — one rule §5.1a did not state

not is legal only as a child of and. A complement needs a universe, and the only one that does not widen is the set its siblings produced: A AND NOT B is "A, less B". A bare NOT B — or A OR NOT B — would have to mean "everyone except…", a broadcast built out of one narrow audience, which is exactly the widening rule 3 forbids. Refused at save, in that sentence.

And the other half: a not contributes no ceiling to the meet. Excluding people cannot widen who an expression reaches, so folding the excluded audience's ceiling in would refuse safe segments — members AND NOT staff would hit meet('members','staff') = null and be rejected despite reaching strictly fewer people than members alone.

Dormancy, three ways, and none of them deletes anything

A rule naming an unregistered trigger, a rule whose channel is gone, and a rule whose segment was deleted are all listed, flagged and left alone (§7.3). In particular audience_segment_id deliberately carries no foreign key: CASCADE would delete an operator's rules and SET NULL would silently fall the rule back to its plain audience column — and that fallback reaches a different set of people, the exact failure §5.1a rule 4 exists to prevent. Deleting a segment a rule still uses is refused in the model, with the count.

Nothing is delivered, and that is visible rather than pretended

A channel's deliver arrives with email in Phase 6 and the inbox in Phase 7. Until then the worker claims the row, finds no deliver, finishes it failed, and the send log says so in as many words. Recording sent would be a lie in the one table whose purpose is answering "did they get it"; leaving it scheduled would mean an IDOC warning queued today arriving three weeks later on the deploy that first ships a mailer. On a real deployment the path is unreachable anyway — core seeds no rules and enabled defaults to 0, so nothing enqueues until 4b's screen exists and an operator uses it.

Verification

npm test — 43 new tests in test/engagementEngine.test.js, all green. The three failures in the suite (routes.manifest, routes.guards, the committed manifest) are the known Windows-only CRLF artifacts and fail identically on a clean edge — verified with git stash -u in the same session.
12 more against a real MariaDB 11.8 (test/engagementEngineSql.test.js), including one that pins the foundRows behaviour that caused defect 2 so it cannot come back.
schema.sql applied and replayed against a throwaway database — idempotent, and uq_engo_dedupe verified four-column on the server.
The whole path exercised end to end against a live database: per-subject cooldowns (house-4001 cools, house-4002 goes through), condition filtering, the CAS claim, the send log's honest failure detail, and a rule going dormant when its module unregisters.

No routes, so no swagger or route-manifest regeneration.

AI-assisted: written with Claude Code (Opus)

🤖 Generated with Claude Code

Engagement **Phase 4**, split **4a / 4b** at the org lead's direction. Companion docs PR: **docs#183**. This is **4a: the engine, server only, with no HTTP surface at all**. It is provably done when a fired trigger produces an outbox row and a send-log entry with no UI in the picture. Admin → Engagement → Rules and the §5.1a segment composition UI are 4b. ## Three decisions settled before any code | | Question | Decision | |---|---|---| | **§7.1 Q2** | multi-instance: `SKIP LOCKED`, or document single-instance? | **Neither**: a compare-and-set claim into the `sending` state §4.2a's ENUM already carried | | **§7.1 Q4** | where the engagement admin surface lives | **its own top-level nav group** — built in 4b | | — | one PR or two | **4a / 4b** | Q2's answer makes the **outbox** safe for two app instances. It does not make the deployment multi-instance — `announceWorker`, `teamDigestWorker`, `teamForumUploadSweep` and `teamActivityPrune` are all still written for one, and widening them is not this phase's scope. What it buys is that the one table that will carry mail is ready for the day it is. ## What lands Five tables (`engagement_rules`, `engagement_audience_segments`, `engagement_cooldowns`, `engagement_outbox`, `engagement_sends`), `src/engagement/{engine,conditions,segments,audiences}.js`, `src/model/engagement/*`, and `utils/engagementWorker.js` wired into `server.js` beside its five siblings. `engagementEmit`'s Phase 2 log line becomes the engine call. **The gate order is the design**, and two placements are load-bearing: - **The G24 ceiling is re-checked at SEND time, not only at save.** The save path already ran the same `ceilings.permits`, so the only way this can fail is the case it exists for — a module upgrade that *narrows* its trigger's declaration underneath a rule saved when it was wider. Without it, a rule written against yesterday's declaration keeps reaching yesterday's population forever. This is the second call site §5.1a promised, not a second implementation. - **The hourly ceiling is checked before the cooldown.** The ceiling is about the rule; the cooldown is about one recipient. A rule at its ceiling should not also burn everybody's cooldown slot on sends that never happen. ## Two defects in ENGAGEMENT.md's own §4, both found by building it **1. §4.2a's global `UNIQUE (dedupe_key)` was data loss, not a style question.** A dedupe key names the *event* — "house 0x4001 entered IDOC" — and one event legitimately becomes many rows: fifty recipients is fifty rows, a rule spanning email and in-app doubles that, a second rule doubles it again. Under a global unique index the **first** insert wins and every other one is silently ignored: ninety-nine recipients dropped by the mechanism meant to stop a replay becoming a second mail. Shipped as `UNIQUE (rule_id, user_id, channel, dedupe_key)`. **2. §4.1's single `INSERT … ON DUPLICATE KEY UPDATE` cooldown claim always passes against this codebase's pool.** Its answer is read out of `affectedRows` — 1 inserted, 2 updated-and-changed, **0 for a duplicate key whose update changed nothing**, that 0 being "still cooling". **The mariadb connector defaults `foundRows: true`**, which makes `affectedRows` report rows *matched* rather than *changed*, and `utils/db.js` does not override it. The no-op returns 1, indistinguishable from a fresh insert. Every cooldown passes, always. It is two statements now — a guarded `UPDATE` with the interval in a WHERE clause, falling back to `INSERT IGNORE` for a first fire — still race-free in both directions. **The second defect is why this PR has two test files.** `engagementEngine.test.js` stubs the five tables and was **green against the broken claim**, because a stub can only agree with whoever wrote it and the same misreading produced both. `engagementEngineSql.test.js` runs the raw statements against a real MariaDB, creates a throwaway database and drops it, and **skips when there is none** so CI stays green without one. *A stub is a fine stand-in for a table and a poor one for a protocol.* ## Segments — one rule §5.1a did not state **`not` is legal only as a child of `and`.** A complement needs a universe, and the only one that does not widen is the set its siblings produced: `A AND NOT B` is "A, less B". A bare `NOT B` — or `A OR NOT B` — would have to mean "everyone except…", a broadcast built out of one narrow audience, which is exactly the widening rule 3 forbids. Refused at save, in that sentence. And the other half: **a `not` contributes no ceiling to the meet.** Excluding people cannot widen who an expression reaches, so folding the excluded audience's ceiling in would refuse safe segments — `members AND NOT staff` would hit `meet('members','staff') = null` and be rejected despite reaching strictly fewer people than `members` alone. ## Dormancy, three ways, and none of them deletes anything A rule naming an unregistered trigger, a rule whose channel is gone, and a rule whose segment was deleted are all **listed, flagged and left alone** (§7.3). In particular `audience_segment_id` deliberately carries **no foreign key**: `CASCADE` would delete an operator's rules and `SET NULL` would silently fall the rule back to its plain `audience` column — and that fallback reaches a *different set of people*, the exact failure §5.1a rule 4 exists to prevent. Deleting a segment a rule still uses is refused in the model, with the count. ## Nothing is delivered, and that is visible rather than pretended A channel's `deliver` arrives with email in Phase 6 and the inbox in Phase 7. Until then the worker claims the row, finds no `deliver`, finishes it `failed`, and the send log says so in as many words. Recording `sent` would be a lie in the one table whose purpose is answering "did they get it"; leaving it `scheduled` would mean an IDOC warning queued today arriving three weeks later on the deploy that first ships a mailer. On a real deployment the path is unreachable anyway — **core seeds no rules and `enabled` defaults to 0**, so nothing enqueues until 4b's screen exists and an operator uses it. ## Verification - `npm test` — **43 new tests** in `test/engagementEngine.test.js`, all green. The three failures in the suite (`routes.manifest`, `routes.guards`, `the committed manifest`) are the known Windows-only CRLF artifacts and **fail identically on a clean `edge`** — verified with `git stash -u` in the same session. - **12 more against a real MariaDB 11.8** (`test/engagementEngineSql.test.js`), including one that pins the `foundRows` behaviour that caused defect 2 so it cannot come back. - `schema.sql` applied and **replayed** against a throwaway database — idempotent, and `uq_engo_dedupe` verified four-column on the server. - The whole path exercised **end to end against a live database**: per-subject cooldowns (house-4001 cools, house-4002 goes through), condition filtering, the CAS claim, the send log's honest failure detail, and a rule going dormant when its module unregisters. No routes, so no swagger or route-manifest regeneration. --- - [x] AI-assisted: written with Claude Code (Opus) 🤖 Generated with [Claude Code](https://claude.com/claude-code)

wtclaude added 1 commit 2026-08-29 13:08:34 +00:00

feat(engagement): the rules engine, cooldowns and outbox (engagement Phase 4a)

PR Checks / bot-tests (pull_request) Successful in 27s

Details

PR Checks / client-build (pull_request) Successful in 30s

Details

PR Checks / server-tests (pull_request) Successful in 2m37s

Details

2079aaf667

Phase 4 of docs/website/ENGAGEMENT.md, split 4a/4b at the org lead's direction.
This is 4a: the engine, server only, with no HTTP surface at all. A fired trigger
now produces outbox rows and send-log entries; Admin - Engagement - Rules and the
segment composition UI are 4b.

Five tables (rules, audience segments, cooldowns, outbox, sends), the sweep
worker, audience resolution, condition evaluation, the grace window and its
cancellation, and the save-path validation 4b's form will call. engagementEmit's
Phase 2 log line becomes the engine call.

Two settled questions this phase was blocked on:

  Q2 (multi-instance) - neither SKIP LOCKED nor documented single-instance: the
  outbox claims each row with a compare-and-set into the 'sending' state the ENUM
  already carried. It makes the outbox safe for two instances, not the deployment.

  Q4 (admin surface) - its own top-level nav group, built in 4b.

Two defects in the plan's own section 4, both found by building it:

  The global UNIQUE(dedupe_key) was data loss. A dedupe key names the EVENT, and
  one event is one row per (rule, user, channel) - so a fifty-person audience
  would have had one row admitted and forty-nine silently ignored. Scoped.

  Section 4.1's single INSERT ... ON DUPLICATE KEY UPDATE cooldown claim always
  passes against this codebase's pool: the mariadb connector defaults
  foundRows:true, so a no-op update reports affectedRows 1 rather than 0. It is
  two statements now, with the interval guard in a WHERE clause.

The second defect is why there is a second test file. The stubbed suite was green
against the broken claim, because a stub can only agree with whoever wrote it;
engagementEngineSql.test.js runs the raw statements against a real MariaDB and
skips when there is none.

Verification: 43 new tests green in engagementEngine.test.js, 12 more against
MariaDB 11.8, and the whole path exercised end to end against a live database -
per-subject cooldowns, conditions, the CAS claim, the send log's honest failure
detail, and dormancy on uninstall. The three pre-existing Windows-only CRLF
failures in the generated-artifact tests are unchanged from clean edge.

Co-Authored-By: Claude <noreply@anthropic.com>

whitlocktech merged commit 4d3f574480 into edge

2026-08-29 13:24:16 +00:00

whitlocktech deleted branch feature/engagement-engine

2026-08-29 13:24:17 +00:00

whitlocktech referenced this issue from a commit

2026-08-29 13:24:18 +00:00

Merge pull request 'feat(engagement): the rules engine, cooldowns and outbox (engagement Phase 4a)' (#170) from feature/engagement-engine into edge

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: RunicGateway/website#170