fix(engagement): claim the seed guard atomically, and let a rule name a digest body #181

wtclaude · 2026-09-01T19:29:51Z

wtclaude commented

2026-09-01 19:29:51 +00:00

The two defects Phase 13's acceptance walk found (docs#203). Neither was visible to any test, and both are in code the workstream shipped.

1 · The one-shot rule-group guard was not a guard

moduleSeeds.seedRuleGroup and coreRules.seedGroup both read their settings stamp, inserted the whole group, and wrote the stamp after the loop. Two instances booting in the same moment both read "absent" and both insert. The walk ended up with 52 module rules where module-uo ships 26, under one stamp — and docker compose up --scale app=2 and a rolling restart both start two instances on purpose, so this is ordinary rather than exotic.

settings.db gains claim(key, value) — the same INSERT IGNORE as seedDefault, reporting its own affectedRows, so exactly one caller can win a key. The atomicity is the PRIMARY KEY's: no transaction, no lock, the same bargain engagementWorker's row claim already makes. Both seeders now claim before inserting.

The trade is unchanged, only its order. A process that dies mid-loop leaves the group stamped and partly seeded; the missing rules are an operator's visit to the "new rule" form. Both functions' own comments already say why that is the better failure — "a duplicate rule is two mails per event", which is precisely what the old ordering produced.

Why nothing caught it. The stubs supplied get and set over a Map, which cannot race, so they agreed with the bug — Phase 4a's foundRows finding in a different costume. The fake is now claim-shaped and decides without yielding, exactly as the table does. Both suites gained a test that runs two seeders under Promise.all:

✔ two instances booting together seed the group once, not twice
✔ two instances booting together seed the Team rules once, not twice

Verified by reverting the fix — the module test then reports every rule inserted twice:

actual:   [ 'demo.house.warning', 'demo.house.warning', 'demo.house.gone', 'demo.house.gone' ]
expected: [ 'demo.house.warning', 'demo.house.gone' ]

2 · A rule that names a `digest` body could not be saved

registries.js checkSeedRule permits digest in as many words — it is the body teamDigestWorker renders for a rule whose email channel an individual set to digest mode, so it never appears in channels and never could — and MODULE_API.md §2.4 tells modules they may point template_keys at notify.digest. engagementRules.model.js then rejected any key that was not one of the rule's channels.

So every rule shipping a digest body was unsaveable from the Rules screen: core's own Team and news rules (coreRules.js lines 62, 84, 138) and sixteen of module-uo's. An operator who opened House — decay warning and pressed Save got a 400 naming a key they had never typed, and the only remedy was deleting the digest body — silently dropping digest support from that rule.

The two validators now agree. Anything that is neither a channel nor digest is still refused, and there is a test for each direction.

No MODULE_API bump

No member is added, removed or changed. The documented contract (§2.4: "a rule may point template_keys at notify.event / inapp.event / notify.digest") is what the code now honours rather than anything new, and the seeding change is core-internal.

Gates

npm test: 1555 pass, 1 fail — and that one is a local-checkout artifact, not this change. engagement-triggers.json is unmodified in git; the manifest test compares the file on disk (CRLF here, via core.autocrlf) against freshly generated output (LF). Regenerating it produces a byte-identical file modulo line endings (git diff reports the path with no content diff), so it is green on CI's Linux checkout. routes:manifest --check: up to date (243 routes) — this adds no route.

AI-assisted: written with Claude Code.

🤖 Generated with Claude Code

The two defects **Phase 13's acceptance walk** found (docs#203). Neither was visible to any test, and both are in code the workstream shipped. ### 1 · The one-shot rule-group guard was not a guard `moduleSeeds.seedRuleGroup` and `coreRules.seedGroup` both read their settings stamp, inserted the whole group, and wrote the stamp **after the loop**. Two instances booting in the same moment both read "absent" and both insert. The walk ended up with **52 module rules where module-uo ships 26**, under one stamp — and `docker compose up --scale app=2` and a rolling restart both start two instances on purpose, so this is ordinary rather than exotic. `settings.db` gains **`claim(key, value)`** — the same `INSERT IGNORE` as `seedDefault`, reporting its own `affectedRows`, so exactly one caller can win a key. The atomicity is the PRIMARY KEY's: no transaction, no lock, the same bargain `engagementWorker`'s row claim already makes. Both seeders now claim before inserting. **The trade is unchanged, only its order.** A process that dies mid-loop leaves the group stamped and partly seeded; the missing rules are an operator's visit to the "new rule" form. Both functions' own comments already say why that is the better failure — *"a duplicate rule is two mails per event"*, which is precisely what the old ordering produced. **Why nothing caught it.** The stubs supplied `get` and `set` over a `Map`, which cannot race, so they agreed with the bug — Phase 4a's `foundRows` finding in a different costume. The fake is now `claim`-shaped and decides without yielding, exactly as the table does. Both suites gained a test that runs two seeders under `Promise.all`: ``` ✔ two instances booting together seed the group once, not twice ✔ two instances booting together seed the Team rules once, not twice ``` Verified by reverting the fix — the module test then reports every rule inserted twice: ``` actual: [ 'demo.house.warning', 'demo.house.warning', 'demo.house.gone', 'demo.house.gone' ] expected: [ 'demo.house.warning', 'demo.house.gone' ] ``` ### 2 · A rule that names a `digest` body could not be saved `registries.js` `checkSeedRule` permits `digest` in as many words — it is the body `teamDigestWorker` renders for a rule whose email channel an individual set to digest mode, so it never appears in `channels` and never could — and `MODULE_API.md` §2.4 tells modules they may point `template_keys` at `notify.digest`. `engagementRules.model.js` then rejected any key that was not one of the rule's channels. So **every rule shipping a digest body was unsaveable from the Rules screen**: core's own Team and news rules (`coreRules.js` lines 62, 84, 138) and sixteen of module-uo's. An operator who opened *House — decay warning* and pressed Save got a 400 naming a key they had never typed, and the only remedy was deleting the digest body — silently dropping digest support from that rule. The two validators now agree. Anything that is neither a channel nor `digest` is still refused, and there is a test for each direction. ### No MODULE_API bump No member is added, removed or changed. The documented contract (§2.4: *"a rule may point `template_keys` at `notify.event` / `inapp.event` / `notify.digest`"*) is what the code now honours rather than anything new, and the seeding change is core-internal. ### Gates `npm test`: **1555 pass, 1 fail** — and that one is a local-checkout artifact, not this change. `engagement-triggers.json` is unmodified in git; the manifest test compares the file on disk (CRLF here, via `core.autocrlf`) against freshly generated output (LF). Regenerating it produces a **byte-identical file modulo line endings** (`git diff` reports the path with no content diff), so it is green on CI's Linux checkout. `routes:manifest --check`: **up to date (243 routes)** — this adds no route. - [x] AI-assisted: written with Claude Code. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

wtclaude added 1 commit 2026-09-01 19:29:53 +00:00

fix(engagement): claim the seed guard atomically, and let a rule name a digest body

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

Details

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

Details

PR Checks / bot-tests (pull_request) Successful in 8m30s

Details

eec7dbf785

Both defects came out of Phase 13's acceptance walk, and neither was visible to
any test.

**1. The one-shot rule-group guard was not a guard.** `seedRuleGroup` and
`coreRules.seedGroup` both read their settings stamp, inserted the whole group,
and wrote the stamp AFTER the loop. Two instances booting in the same moment both
read "absent" and both insert -- the walk ended up with 52 module rules where the
module ships 26. `docker compose up --scale app=2` and a rolling restart both
start two instances on purpose, so this is ordinary rather than exotic.

`settings.db` gains `claim(key, value)`: the same `INSERT IGNORE` as
`seedDefault`, reporting its own `affectedRows`, so exactly one caller can win a
key. The atomicity is the PRIMARY KEY's -- no transaction, no lock, the same
bargain `engagementWorker`'s row claim already makes. Both seeders claim before
inserting.

The trade the code already documented is unchanged, only its order: a process
that dies mid-loop leaves the group stamped and partly seeded, and the missing
rules are an operator's visit to the "new rule" form. A duplicate rule is two
mails per event, for every rule in the group, which both functions' own comments
already call the worse outcome.

**Why no test caught it:** the stubs supplied `get` and `set` over a Map, which
cannot race, so they agreed with the bug -- Phase 4a's `foundRows` finding in a
different costume. The fake is now `claim`-shaped and decides without yielding,
exactly as the table does, and both suites gained a test that runs two seeders
with `Promise.all` and asserts one insert each. Verified by reverting the fix:
the module test then reports every rule inserted twice.

**2. A rule that names a `digest` body could not be saved.** `registries.js`
`checkSeedRule` permits `digest` in as many words -- it is the body
`teamDigestWorker` renders for a rule whose email channel an individual set to
digest mode, so it never appears in `channels` and never could -- and
MODULE_API.md 2.4 tells modules they may point `template_keys` at
`notify.digest`. `engagementRules.model.js` then rejected any key that was not
one of the rule's channels.

So every rule shipping a digest body answered an operator who opened it and
pressed Save with a 400 naming a key they had never typed: core's own Team and
news rules, and sixteen of module-uo's. The only way to save was to delete the
digest body, silently dropping digest support from that rule. The two validators
now agree; anything that is neither a channel nor `digest` is still refused, with
a test for each direction.

No MODULE_API bump: no member is added, removed or changed, and the documented
contract is what the code now honours rather than something new.

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

whitlocktech approved these changes 2026-09-01 19:36:15 +00:00

whitlocktech scheduled this pull request to auto merge when all checks succeed 2026-09-01 19:36:22 +00:00

whitlocktech merged commit e59a68c152 into main

2026-09-01 19:40:00 +00:00

whitlocktech deleted branch fix/engagement-seed-claim-and-digest

2026-09-01 19:40:01 +00:00

whitlocktech referenced this issue from a commit

2026-09-01 19:40:02 +00:00

Merge pull request 'fix(engagement): claim the seed guard atomically, and let a rule name a `digest` body' (#181) from fix/engagement-seed-claim-and-digest into main

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: RunicGateway/website#181

fix(engagement): claim the seed guard atomically, and let a rule name a digest body #181

1 · The one-shot rule-group guard was not a guard

2 · A rule that names a digest body could not be saved

No MODULE_API bump

Gates

fix(engagement): claim the seed guard atomically, and let a rule name a `digest` body #181

2 · A rule that names a `digest` body could not be saved