docs(engagement): Phase 14 as built — retention

The last phase of the workstream. ENGAGEMENT.md gets Phase 14's as-built
(the six org-lead decisions, three findings, the acceptance line discharged
and one rig trap), and its header now reads COMPLETE rather than "Phase 14
is scoped and not started".

BACKEND_DESIGN.md gets the reference: a new "Engagement retention" section
with the horizon table, why the outbox sweep is terminal-only, why each
floor is a correctness bound rather than a UI nicety, why suppressions do
not expire, and why this one got a screen when team_activity and
user_notifications did not. Plus the three new routes in the admin table and
the address_hash reversal on GET /suppressions.

Three findings worth not re-deriving:

  * the per-row purge could not be built without reversing Phase 9's
    decision to strip address_hash — the row had no handle, and the existing
    lift asks for an address the screen has never shown.
  * reclaimStale could not give up, so an interrupted send never became
    terminal and no retention sweep could ever have bounded the outbox.
  * the cooldown warning is unreachable through the UI by construction
    (MAX_COOLDOWN_SECONDS is smaller than the horizon's floor). It is not
    dead: it catches a hand-edited row, and a future raise of that ceiling.

And one the phase text got wrong in the other direction: the Play Data
Safety answers correctly did NOT move, because deploy-engagement is
deployment-scoped and PLAY_DATA_SAFETY.md is generated from the app-scoped
entries only.

Code: website#TBD.

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
2026-09-01 15:41:13 -05:00
parent 6601e410f4
commit fdd5bf36f7
2 changed files with 138 additions and 10 deletions

View File

@@ -1,11 +1,12 @@
# The Engagement System — findings and plan
**Status:** design of record. **The workstream is BUILT, CUT OVER and ACCEPTED (2026-09-01).**
Phases 0 through 13 are done and on `main` in every repository, and **Phase 13's acceptance walk
**Status:** design of record. **The workstream is COMPLETE (2026-09-01).**
Phases 0 through 14 are done and on `main` in every repository, and **Phase 13's acceptance walk
passed** — a clean install from `main`, released artefacts only, one email and one in-app item to the
linked owner and nothing to anyone else. It found two defects in shipped core; both are recorded in
Phase 13's as-built and neither is fixed there. **Phase 14 (retention) is scoped and not started.** Each phase carries its own as-built
section below, and the PRs are named there rather than here.
Phase 13's as-built and both were fixed afterwards (website#181). **Phase 14 (retention) is BUILT**
the last phase, and the one that gave the engagement schema a bound. Each phase carries its own
as-built section below, and the PRs are named there rather than here.
The cutover cut `link` **v2.1.0**, the plugin overlay **v1.1.0**, `module-uo` **v1.1.0** and the
paired bundle **2026.09.01** (protocol **5**); `MODULE_API_VERSION` is **1.9.0**. Two of Phase 13's
@@ -23,7 +24,9 @@ of Phase 8**. Q1's answer added a whole phase (**Phase 1b**, unique email addres
the phase's size split **Phase 4 into 4a and 4b**; **Q9** (core's own `news.post` emitter) was
answered **2026-08-31 at the start of Phase 11**, along with five further decisions that changed what
that phase ships — see its own decision block. Per CLAUDE.md § Conventions, no implementation starts
without the org lead's approval of the phase it belongs to, **and that still holds for Phase 14.**
without the org lead's approval of the phase it belongs to; **Phase 14's six decisions were settled
on 2026-09-01 before any of its code**, and two of the four horizon answers widened it — see its
as-built.
**Branching:** every phase landed on **`edge`** in its repo, and `main` was touched once, by the
cutover (Phase 13). §6.0a records the blocking precondition it opened with — six `edge` branches
@@ -3869,9 +3872,11 @@ engagement system a headline capability — and Phase 12's second finding became
### Phase 14 — Retention: the engagement schema has no sweep
**Status: scoped, not started.** Phase 12 found this and recorded it without fixing it (finding 2 of
**Status: BUILT, 2026-09-01.** Phase 12 found this and recorded it without fixing it (finding 2 of
its as-built), on the grounds that a sweep is a `website` change and outside a documentation phase.
Nothing after it picked the finding up, so it is stated here as the phase it always was.
Nothing after it picked the finding up, so it was stated here as the phase it always was — and this
is that phase. Six decisions were settled by the org lead before any code; the as-built is at the end
of this section.
**Four tables grow without bound**, and they are not one problem with one horizon:
@@ -3907,6 +3912,75 @@ longest enabled rule's cooldown rather than picked; `/privacy` and the Play answ
from the inventory; and a rig run shows the sweep deleting terminal rows while leaving a `scheduled`
outbox row and an in-window send-log row alone.
#### Phase 14 as built
**The six decisions, settled by the org lead before any code.** Four horizons were put with the facts
the tree supplied, and two of the four answers went past what was offered:
1. **`engagement_cooldowns` — 30 days**, plus the runtime guard. (`MAX_COOLDOWN_SECONDS` is a
validated 86 400, so even 2 days is provably safe; 30 was chosen for headroom.)
2. **`engagement_outbox` — terminal rows only, 30 days, plus a stuck-`sending` reaper.** The reaper
half turned out to be half-built already — see finding 2.
3. **`engagement_sends` — admin-configurable, 90 / 180 / 365 / custom.** This is the answer that
widened the phase: none of the three offered options had a UI, and the other two retention workers
in this codebase (`teamActivityPrune`, `userNotificationsPrune`) keep their horizons in invisible
`settings` rows. It bought the phase a screen, and the screen took the other two horizons with it —
"what does this deployment keep" is one question.
4. **`engagement_suppressions` — never expires, plus an admin purge.** The purge half also went past
the offer, and cost a Phase 9 decision — see finding 1.
**As built.** `utils/engagementRetentionPrune.js` (one worker, three sweeps, batched 1000 × 50, each
table's failure caught on its own), `model/engagement/engagementRetention.model.js` (the policy, its
bounds and the cooldown check), two `.db` additions (`outbox.pruneTerminal`, `sends.prune`) and a
`limit` on the cooldown prune that already existed and had never had a caller. Three routes
(`GET`/`PUT /admin/engagement/retention`, `DELETE /admin/engagement/suppressions/by-hash/:hash`), a
new Admin → Engagement → **Retention** screen, a per-row **Lift** button on Suppressions, and one line
under the Send Log's pager. Two indexes (`idx_engo_sweep`, `idx_engs_sweep`) — every existing index on
those tables has `created_at` in *second* position, which serves a per-rule window and is useless to a
whole-table horizon. Reference: `BACKEND_DESIGN.md` § *Engagement retention*.
**Three findings.**
1. **The per-row purge cost a Phase 9 decision, and there was no way to have it otherwise.** Phase 9
deliberately stripped `address_hash` from every listed row — a sha256 of every address on the
deployment is an offline dictionary attack waiting to be run — and the consequence, which its own
screen shows, is that the only way out of the list was a `window.prompt` asking the operator to
retype the full address. **They do not have it**: the screen stores and shows a mask. So the row
had no handle at all, and a per-row button was not a UI change but a reversal. Taken knowingly:
the route is admin-only and an admin can already suppress and unsuppress any address they can
name, so the hash grants no capability they lack. `GET /sends` still strips its own.
2. **`reclaimStale` could not give up, so the outbox sweep would not have been a bound.** The
stuck-`sending` reaper decision 2 asked for already existed and already ran every worker tick —
but it returned *every* stale row to `scheduled`, and `MAX_ATTEMPTS` is consulted only on a
graceful `retry` outcome. A send that killed the process mid-flight therefore cycled
sending → scheduled → sending **forever**: never terminal, therefore never eligible for the
terminal-only sweep this phase was adding. Fixed by failing an exhausted row *before* reclaiming
the rest; the order is the fix, and `engagementRetentionSql.test.js` runs the wrong order
deliberately to show what it avoids.
3. **The cooldown warning is unreachable through the UI, by construction.** `MAX_COOLDOWN_SECONDS`
(86 400) is smaller than the cooldown horizon's floor (2 days), so no rule that can be *saved* can
trip the guard. It is not therefore dead: it fires for a hand-edited row, and it is the thing that
catches a future raise of `MAX_COOLDOWN_SECONDS` that forgets this floor. Proved live by setting a
5-day cooldown by hand — the screen and the sweep both warned, and the sweep ran anyway.
**The acceptance line, discharged.** All four tables have a stated policy (three sweeps and one
recorded refusal). The cooldown horizon is checked rather than picked. `/privacy` is regenerated from
`collection.mjs` — **and the Play answers correctly did not move**, which the phase text did not
expect: `deploy-engagement` is `scope: 'deployment'`, and `PLAY_DATA_SAFETY.md` is generated from the
`app`-scoped entries only, because Play asks what the *app* collects rather than what a self-hosted
deployment keeps. `npm run play:datasafety` rewrites the file byte-identically and
`check:datasafety` stays green. The rig run happened twice: `engagementRetentionSql.test.js`
against a throwaway database (7 tests), and the live stack, where a 90-day-old `cancelled` row was
swept and a 90-day-old `scheduled` row survived.
**One rig trap worth not re-deriving.** The first live run appeared to delete the `scheduled` row —
the acceptance case, apparently failing. It had not: the outbox **worker** was running in the same
server, the planted row's `due_at` was 90 days in the past, so the worker legitimately claimed and
sent it before the sweep saw it. **Plant the scheduled row with a FUTURE `due_at`**, or the worker,
not the sweep, is what the test is measuring.
---
### Sequencing