fix(engagement): two defects the Phase 11b live walk found in core #179

wtclaude · 2026-09-01T12:14:19Z

wtclaude commented

2026-09-01 12:14:19 +00:00

Core's half of Phase 11b's acceptance walk. Pairs with Module-uo# (four more defects and the 26th trigger), servuo-plugins# (the market sweep) and docs# (the three decisions).

Both defects here are invisible to a fixture and loud on a real database — which is the argument for the walk being the phase's acceptance rather than a formality.

1. Every module-seeded rule failed to insert

checkSeedRule validates max_sends_per_hour — Q3's hard ceiling, the thing that keeps a misconfiguration from becoming a mail storm — and then drops it from the normalized rule it returns. The column is NOT NULL, so the first boot with module-uo installed produced 25 of these and seeded nothing:

ERROR [engagement] module rule seed failed {"owner":"uo","group":"triggers-v1",
  "trigger":"uo.house.collapsed","message":"Column 'max_sends_per_hour' cannot be null"}

The existing test asserted the rejection of a bad ceiling (max_sends_per_hour: 0 → error) and never that a good one survives normalization. Those are different bugs and the first test cannot see the second.

The new test is written against engagementRules.db.insert's own column list rather than against the one field, because the next field added to the declaration is the next one that can be forgotten here.

One operational note. A group is stamped even when it seeds partially (deliberate — re-running would duplicate the rules that did insert). Nothing outside a local rig has ever booted this code, so no deployment carries a triggers-v1 stamp with an empty group; if one did, it would need the settings row cleared, not a new group key.

2. A rule with a cooldown delivered on exactly ONE of its channels

cooldownsDb.claim is called inside the engine's per-channel loop and its key was (rule, user, subject). So the first channel of a rule claimed the cooldown and every later one came back false — reported as cooled, which reads in the log exactly like the cooldown doing its job:

event dispatched {"trigger":"uo.house.idoc_warning","rules":1,
                  "enqueued":1,"cooled":1}     ← inapp enqueued, email cooled

inapp is ranked first on purpose (CHANNEL_ORDER — so push can reference the inbox row it writes), so this is not a coin-flip: a rule naming email and in-app delivers the inbox item and silently never the mail. Core's own seeded news.post rule has that shape — three channels, an hour's cooldown — and so does every one of the sixteen in-universe bodies Phase 11b wrote, each of which was unreachable behind its own in-app twin.

The org lead's decision 12: channel joins the cooldown key. The alternative — claiming once per user before the channel loop — was refused because it makes the cooldown a per-occasion limit, and an operator who sets "one a day about this house" means one mail and one inbox item, not one of the two. A cooldown is per delivery.

The migration, and why it is guarded the way it is

ALTER TABLE engagement_cooldowns ADD COLUMN IF NOT EXISTS channel VARCHAR(32) NOT NULL DEFAULT '';
SET @engc_key_has_channel := ( … information_schema.STATISTICS … );
SET @sql := IF(@engc_key_has_channel = 0, 'ALTER TABLE … DROP PRIMARY KEY, ADD PRIMARY KEY …', 'DO 0');
PREPARE stmt FROM @sql; EXECUTE stmt; DEALLOCATE PREPARE stmt;

MariaDB has no conditional form of a primary-key change, and schema.sql is replayed on every boot — so the unguarded version would come up once and fail the entire schema step on every boot after that. The guard reads the KEY out of information_schema rather than the column's existence, because ADD COLUMN IF NOT EXISTS can succeed while the key change does not. Same shape as the announce_job_legs backfill a few hundred lines up.

Rows written before the migration keep channel = '' and expire on their own interval — one stale cooldown per (rule, user, subject), which is a better trade than dropping the table and letting a storm through the window.

Verification

1551 core tests green, 0 failing.
Both new tests were verified by reverting each fix in turn: the seed test fails with normalized rule is missing "max_sends_per_hour", the engine test with the email row absent.
The migration ran against a live MariaDB carrying the old key: PRIMARY KEY (rule_id, user_id, subject_key, channel) afterwards, and the 26 module rules seeded on the same boot.
On the rig, uo.house.idoc_warning then went enqueued: 2, cooled: 0 — both channels — and a second transition inside the day went enqueued: 0, cooled: 2.

AI-assisted: written with Claude Code.

🤖 Generated with Claude Code

Core's half of Phase 11b's acceptance walk. Pairs with **Module-uo#** (four more defects and the 26th trigger), **servuo-plugins#** (the market sweep) and **docs#** (the three decisions). Both defects here are invisible to a fixture and loud on a real database — which is the argument for the walk being the phase's acceptance rather than a formality. ## 1. Every module-seeded rule failed to insert `checkSeedRule` validates `max_sends_per_hour` — Q3's hard ceiling, the thing that keeps a misconfiguration from becoming a mail storm — and then **drops it from the normalized rule it returns**. The column is `NOT NULL`, so the first boot with module-uo installed produced 25 of these and seeded nothing: ``` ERROR [engagement] module rule seed failed {"owner":"uo","group":"triggers-v1", "trigger":"uo.house.collapsed","message":"Column 'max_sends_per_hour' cannot be null"} ``` The existing test asserted the **rejection** of a bad ceiling (`max_sends_per_hour: 0` → error) and never that a good one survives normalization. Those are different bugs and the first test cannot see the second. The new test is written against `engagementRules.db.insert`'s own column list rather than against the one field, because the next field added to the declaration is the next one that can be forgotten here. > **One operational note.** A group is stamped even when it seeds partially (deliberate — re-running would duplicate the rules that did insert). Nothing outside a local rig has ever booted this code, so no deployment carries a `triggers-v1` stamp with an empty group; if one did, it would need the settings row cleared, not a new group key. ## 2. A rule with a cooldown delivered on exactly ONE of its channels `cooldownsDb.claim` is called **inside** the engine's per-channel loop and its key was `(rule, user, subject)`. So the first channel of a rule claimed the cooldown and every later one came back `false` — reported as `cooled`, which reads in the log exactly like the cooldown doing its job: ``` event dispatched {"trigger":"uo.house.idoc_warning","rules":1, "enqueued":1,"cooled":1} ← inapp enqueued, email cooled ``` `inapp` is ranked first **on purpose** (`CHANNEL_ORDER` — so `push` can reference the inbox row it writes), so this is not a coin-flip: a rule naming email and in-app delivers the inbox item and *silently never the mail*. Core's own seeded `news.post` rule has that shape — three channels, an hour's cooldown — and so does every one of the sixteen in-universe bodies Phase 11b wrote, each of which was unreachable behind its own in-app twin. **The org lead's decision 12: `channel` joins the cooldown key.** The alternative — claiming once per user before the channel loop — was refused because it makes the cooldown a per-*occasion* limit, and an operator who sets "one a day about this house" means one mail *and* one inbox item, not one of the two. A cooldown is per delivery. ### The migration, and why it is guarded the way it is ```sql ALTER TABLE engagement_cooldowns ADD COLUMN IF NOT EXISTS channel VARCHAR(32) NOT NULL DEFAULT ''; SET @engc_key_has_channel := ( … information_schema.STATISTICS … ); SET @sql := IF(@engc_key_has_channel = 0, 'ALTER TABLE … DROP PRIMARY KEY, ADD PRIMARY KEY …', 'DO 0'); PREPARE stmt FROM @sql; EXECUTE stmt; DEALLOCATE PREPARE stmt; ``` **MariaDB has no conditional form of a primary-key change**, and `schema.sql` is replayed on every boot — so the unguarded version would come up once and fail the entire schema step on every boot after that. The guard reads the KEY out of `information_schema` rather than the column's existence, because `ADD COLUMN IF NOT EXISTS` can succeed while the key change does not. Same shape as the `announce_job_legs` backfill a few hundred lines up. Rows written before the migration keep `channel = ''` and expire on their own interval — one stale cooldown per (rule, user, subject), which is a better trade than dropping the table and letting a storm through the window. ## Verification - **1551 core tests green, 0 failing.** - Both new tests were verified by reverting each fix in turn: the seed test fails with `normalized rule is missing "max_sends_per_hour"`, the engine test with the email row absent. - The migration ran against a live MariaDB carrying the old key: `PRIMARY KEY (rule_id, user_id, subject_key, channel)` afterwards, and the 26 module rules seeded on the same boot. - On the rig, `uo.house.idoc_warning` then went `enqueued: 2, cooled: 0` — both channels — and a second transition inside the day went `enqueued: 0, cooled: 2`. --- AI-assisted: written with Claude Code. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

wtclaude added 1 commit 2026-09-01 12:14:21 +00:00

fix(engagement): two defects the Phase 11b live walk found in core

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

Details

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

Details

PR Checks / server-tests (pull_request) Successful in 5m30s

Details

c8d45733b6

Both are invisible to a fixture and loud on a real database, which is why the
walk is the phase's acceptance rather than a formality.

1. registerEngagementSeeds validated `max_sends_per_hour` and then dropped it
   from the normalized rule. The column is NOT NULL, so every one of the 25
   module-seeded rules failed to insert at boot. The registry test asserted the
   REJECTION of a bad ceiling and never that a good one survives; it now asserts
   the normalized rule against `engagementRules.db.insert`'s own column list, so
   the next field added is covered the day it is added.

2. The cooldown claim runs inside the engine's per-channel loop and its key was
   (rule, user, subject). So the first channel of a rule claimed the cooldown and
   every later one was reported as cooled -- and `inapp` is ranked first
   deliberately, so a rule naming email + in-app delivered the inbox item and
   silently never the mail. Core's own `news.post` rule has that shape. Phase
   11b's decision 8 requires the letter and the inbox item to fire together.

   `channel` joins the PRIMARY KEY (the org lead's decision 12: a cooldown is per
   delivery, not per occasion). Migrated in place behind an information_schema
   guard, because MariaDB has no conditional form of a key change and replaying
   schema.sql would otherwise fail on every boot after the first.

1551 core tests green; both new tests verified by reverting each fix in turn.

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

whitlocktech merged commit 52eac24d17 into edge

2026-09-01 12:32:31 +00:00

whitlocktech deleted branch fix/engagement-live-walk-core

2026-09-01 12:32:32 +00:00

whitlocktech referenced this issue from a commit

2026-09-01 12:32:34 +00:00

Merge pull request 'fix(engagement): two defects the Phase 11b live walk found in core' (#179) from fix/engagement-live-walk-core into edge

wtclaude referenced this pull request

2026-09-01 13:46:23 +00:00

feat(engagement): the engagement system — cutover 3 of 7 (`edge` → `main`) #180

Sign in to join this conversation.