diff --git a/website/BACKEND_DESIGN.md b/website/BACKEND_DESIGN.md index c55b761..a6f832d 100644 --- a/website/BACKEND_DESIGN.md +++ b/website/BACKEND_DESIGN.md @@ -553,6 +553,7 @@ core's. | `team_forum_moderation` | append-only, per Team, recording `actor_role` — WHICH authority was exercised. Deliberately not merged with `mod_actions`/`appeals`, which is Discord-sanction-shaped | | `team_forum_uploads` | attribution for `uploads` mode: who uploaded what, when, how big, and to which post. Also the sweep's worklist | | `team_notification_prefs` | per-Team notification preference (phase 6). **Opt-out for push, opt-IN for email** — `muted` defaults 0 and `email_mode` defaults `'off'`, so the two sinks default opposite ways and the asymmetry lives here rather than in a condition anyone has to remember. Team scoping lives in this table and in the recipient computation, never in a stream id. `last_digest_at` is the digest's only state and the worker is its only writer | +| `team_integration_config` | where a Team's notifications go on another platform (phase 8). One row per (platform, Team) plus a **deployment-wide default** whose `team_id` is NULL — expressed with a generated `team_key AS IFNULL(team_id, 0)` in the unique key, because a NULL cannot live in a primary key and the default row is the base case of the whole override mechanism. `members_ack` is a **precondition, not a preference**: forum posts and announcements are members-only always, core cannot see a channel's permissions, so enabling one requires an attributed operator acknowledgement that the destination is restricted — and changing the channel clears it | | `content_reports` | member-raised abuse reports (phase 5). **Not a `team_*` table and not named for the forum** — `target_type` is a plain VARCHAR so a wiki page or a news comment becomes a value rather than a table. Team forum content is only the first consumer | **Core had no user-facing report flow of any kind before `content_reports`.** `moderation`, diff --git a/website/TEAMS.md b/website/TEAMS.md index 2ed2e6a..86b4ea9 100644 --- a/website/TEAMS.md +++ b/website/TEAMS.md @@ -1776,10 +1776,20 @@ ephemeral "link your account for more" — see §9 answer 5. ### 7.2 Notifications bridge +> **Amended after building it (phase 8, 2026-08-18).** The shape below is what was designed; five +> things about it did not survive contact with the tree, and the amendments are inline. The largest +> is that **this section's own visibility gate has no data source and cannot have one** — see "The +> gate, as built" below. The phase entry in Part 12 carries the full list. + The same Team events as §6, delivered to a second consumer. Core emits each Team notification to an internal fan-out with two subscribers: push (§6) and the integration bridge. **Not a second pipeline** — one event, two deliveries. +> **As built**, there is no new fan-out object: `utils/teamNotify.js` already computed the recipient +> set once and handed the event to push and to email, so the bridge is a **third sink in that same +> file** rather than a subscriber to something new. `utils/teamBridge.js` is the sink; the file that +> calls it is unchanged in structure. + ```sql CREATE TABLE IF NOT EXISTS team_integration_config ( platform VARCHAR(32) NOT NULL, -- 'discord' @@ -1792,10 +1802,33 @@ CREATE TABLE IF NOT EXISTS team_integration_config ( ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; ``` +> **This DDL cannot hold its own default row.** MariaDB coerces every `PRIMARY KEY` column to +> `NOT NULL`, so `team_id NULL` — the deployment-wide default that every override overrides — is +> unrepresentable, and the whole mechanism has no base case. **As built:** a surrogate `id` primary +> key, a generated `team_key INT AS (IFNULL(team_id, 0)) STORED` carrying +> `UNIQUE KEY (platform, team_key)`, and a real `FOREIGN KEY (team_id) … ON DELETE CASCADE` that the +> original had no room for — without it a deleted Team leaves its configuration behind for whichever +> Team next lands on that id. The generated-column trick is the one `teams.active_key` and +> `content_reports.open_marker` already use. Three further columns carry the gate: `members_ack`, +> `members_ack_by` and `members_ack_at`. + Admin-configurable per event type, globally and per Team (a per-Team row overrides the `team_id IS NULL` default). Delivered via `POST /internal/team-notify` on the bot, best-effort, never throwing — identical to `announce` and `mod-reverse`. +> **`announce` and `mod-reverse` are not the same thing.** `announce` rides `announce_jobs` with +> backoff, retries and a per-leg retry button in the admin panel; `mod-reverse` is a one-shot call +> that records failure and stops. **The bridge is one-shot.** A news post is a durable artifact whose +> Discord copy is expected to exist; a Team notification is the moment it describes, and one that +> arrives twenty minutes late is worse than one that never arrives. A second job table and a second +> worker is a great deal of machinery to buy the opposite outcome. +> +> **The admin surface is its own panel under Admin → Teams**, beside the forum settings, and not an +> extension of the Discord Bot panel — phase 10 makes the platform a registry lookup, and what should +> change then is what fills the panel, not where it is. It is **admin-only**, the one such corner of a +> staff-wide router: configuring where a Team's content leaves the site for is deployment +> configuration rather than the §2.9 kind of decision a moderator files a request for. + **A Discord message carries content; a push tickle does not.** Stated explicitly because the two look like the same event and are not: ntfy is an untrusted relay reached by an unguessable topic, so the tickle is content-free by design; the Discord server is an operator-configured, trusted destination @@ -1803,6 +1836,45 @@ where an empty "something happened, go look" message would be useless. What *is* allowlist discipline — an event is bridged only if its `visibility` is `public`, or its destination channel is configured for a members-only Team context. +#### The gate, as built + +**Neither half of that last sentence has a data source, and neither can have one.** + +- The four `team.*` streams carry **no `visibility`**. Only `team_activity` rows do, and a + notification is not an activity row. +- Forum threads have **no public/members column**, because a forum is members-only by construction — + every thread in it sits behind `team_forum_grants`. So §7.2's own example configuration, + `['team.announcement','team.forum.post']`, names exactly the two events that can never be public. +- Core **cannot see a Discord channel's permissions**, so "configured for a members-only Team + context" is not a fact core can check. Only the operator can see it. + +So the gate becomes an **attributed acknowledgement**: enabling an event that carries members-only +content requires an explicit confirmation that the destination channel is restricted to that Team's +members, recorded with who gave it and when — the same shape `teams_forum_uploads_ack` uses for the +image policy (§5.5.5). Four properties make it a gate rather than a checkbox: + +1. **It is a precondition, not a preference.** A save that would enable a members-only event without + it is refused **422**, not accepted-and-quietly-degraded. A configuration that silently does less + than it says is worse than one that will not save. +2. **It is re-asked at delivery**, not only at the save, so a row that loses the tick — an admin + repoints it, or a future change reclassifies a stream it already carries — stops carrying those + events immediately rather than at the next save. +3. **Changing the channel clears it.** An acknowledgement is about a *destination*; it cannot survive + the destination changing underneath it, or an operator could confirm a private channel and then + repoint the row at a public one while keeping the permission granted for somewhere else. +4. **A roster-only bridge needs no acknowledgement at all**, and a *disabled* row may carry forum + events without one — drafting a configuration is not publishing to a channel, and a dialog that + appears on saves that did not need it is one people learn to click through. + +Two smaller consequences of the same asymmetry: + +- **The author exclusion stops at the channel.** Push and email both subtract the post's author; the + bridge does not. Excluding is a per-recipient idea, and a channel has no per-recipient anything — + suppressing the message because the author reads that channel would deprive everyone else in it. +- **A roster event carries a count and never a name.** The sync notifies once per run rather than + once per member (§6.2), so a count is all the caller holds. It is also all it should say: a + character name is game-sourced text screened for a *page*, not for a channel. + ### 7.3 One voice channel per Team **Shape.** One voice channel per qualifying Team, under a single shared parent category @@ -2556,10 +2628,86 @@ static commands. It is not defensible now that it merges a pulled set into a sin registration and runs the interaction path, and phases 8 and 9 add more. `bot/test/` and a `bot-tests` job replace `bot-install`. -### Phase 8 — Discord: notifications bridge (`website` + `bot`) +### Phase 8 — Discord: notifications bridge (`website` + `docs`) — **DONE 2026-08-18** -`team_integration_config`, the internal fan-out with push and bridge as two consumers, -`POST /internal/team-notify`, the admin per-event configuration. +`team_integration_config`, the bridge as a third sink beside push and email, `POST +/internal/team-notify`, and the admin per-event configuration. + +**Ships:** a Team's forum posts, announcements and roster changes arriving in a Discord channel the +operator chose, per Team or deployment-wide. + +**ONE code repo, not the plan's `website` + `bot`.** `bot` is a workspace inside `website`, the same +correction phase 7 made — but unlike phase 7 nothing here belongs to a module, so `module-uo` is +untouched: the four streams are core's own and the bridge reads core's own forum. `MODULE_API_VERSION` +does not move. + +**Walked on the live rig before the PRs opened**, per the order phase 5 set. + +#### Five things the tree disagreed with §7.2 about + +1. **`PRIMARY KEY (platform, team_id)` cannot hold the default row.** MariaDB coerces every primary + key column to `NOT NULL`, so `team_id NULL` — the deployment-wide default, and the base case of the + whole override mechanism — is unrepresentable. As built: a surrogate `id`, a generated + `team_key AS (IFNULL(team_id, 0)) STORED` in the unique key, and the foreign key the original DDL + had no room for. Same idiom as `teams.active_key` and `content_reports.open_marker`. +2. **The visibility gate has no data source on either side, and cannot have one.** §7.2 bridges an + event only if "its `visibility` is `public`, or its destination channel is configured for a + members-only Team context". The four `team.*` streams carry no visibility — only `team_activity` + rows do, and a notification is not an activity row — and forum threads have no public/members + column because a forum is members-only by construction, everything in it sitting behind + `team_forum_grants`. So §7.2's own example config, `['team.announcement','team.forum.post']`, + names exactly the two events that are never public. Nor can core see a Discord channel's + permissions to check the other half. + + **As built: an attributed operator acknowledgement**, `members_ack` / `members_ack_by` / + `members_ack_at`, in the shape `teams_forum_uploads_ack` already uses. Enabling a members-only + event without it is refused **422** rather than dropped at delivery, because a configuration that + silently does less than it says is worse than one that will not save. It is re-asked at delivery as + well as at the save, so a row that loses the tick stops carrying those events at once — and + **changing the channel clears it**, since an acknowledgement is about a destination and cannot + survive the destination changing underneath it. +3. **"Identical to `announce` and `mod-reverse`" names two different things.** `announce` rides + `announce_jobs` with backoff, retries and a per-leg retry button; `mod-reverse` is one-shot. The + bridge is **one-shot**: a news post is a durable artifact whose Discord copy is expected to exist, + while a Team notification is the moment it describes, and a message arriving twenty minutes after + the conversation moved on is worse than one that never arrives. A bot that is down drops it, which + is the deal the push tickle already takes. +4. **The author exclusion stops at the channel.** Push and email both subtract the author; the bridge + does not. Excluding is a per-recipient idea and a channel has no per-recipient anything — + suppressing the message because the author happens to read that channel would deprive everyone + else in it. +5. **A roster event has a count and no name.** The sync notifies once per run rather than once per + member (§6.2), so a count is all the caller holds; it is also all it should say. `memberJoined` + grew an optional `{ count }` **for the bridge only** — a channel has no app on the other end to + pull anything after a content-free nudge — and the tickle beside it is unchanged. + +#### Where the admin surface lives, and why it is not in the Discord panel + +Its own panel under **Admin → Teams**, beside the forum settings, rather than an extension of +`DiscordBotAdmin`. Phase 10 replaces "Discord" with whatever the capability registry declares; what +should change then is what fills the panel, not where an operator goes to find it. It is the one +**admin-only** corner of a staff-wide router: this is not the §2.9 kind of decision a moderator files +a request for, it is deployment configuration, and it sits with the role that already holds the bot +token. + +#### What the rig proved, and the two defects it found + +Real ServUO + real sidecar (protocol 4) + the app with module-uo installed, with a fake standing in +for Discord. It proved the default row governing a Team with no row of its own, a per-Team override +beating it (including an override that switches the bridge OFF for one Team while the default stays +on), the 422 on an unacknowledged forum bridge, the acknowledgement clearing on a repoint, forums +switched off silencing the bridge along with the push, and a bot that is down costing the forum reply +nothing. + +**Both defects came out of tests written against the rig's shapes.** A re-acknowledgement given for a +NEW channel kept the OLD attribution — the column was already 1, so "freshly acknowledged" read false +and the row went on naming whoever vetted the previous destination, which is the entire audit value of +the column. And the embed description was clamped to Discord's limit **before** the heading was +prepended, producing a description one heading over the limit; discord.js rejects that outright, so an +over-long forum post would not have arrived at all rather than arriving truncated. + +**Not done here.** No real Discord guild was involved — `channels.fetch` and a real `channel.send` +are the two things this walk could not exercise, the same gap phase 7 recorded for `REST.put`. ### Phase 9 — Discord: voice channels (`website` + `bot`)