diff --git a/website/BACKEND_DESIGN.md b/website/BACKEND_DESIGN.md index a6f832d..a5e7668 100644 --- a/website/BACKEND_DESIGN.md +++ b/website/BACKEND_DESIGN.md @@ -554,6 +554,7 @@ core's. | `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 | +| `team_integrations` | a Team's provisioned resource on another platform — today its Discord **voice channel and the role that opens it** (§7.3, phase 9). Both refs on one row because they are one lifecycle: a role for a channel that no longer exists is a badge for nowhere. `state` is core's BELIEF about the platform, never the platform's answer — the reconciler writes what it just did and the next pass re-derives the truth. A Team that stops qualifying goes to `pending_removal` with `remove_after` rather than being deleted at once, so a Team hovering around the size threshold does not delete-and-recreate its channel and change its id. `synced_at` is separate from `updated_at`, which moves whenever core writes a belief including an error | | `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 86b4ea9..650ab2c 100644 --- a/website/TEAMS.md +++ b/website/TEAMS.md @@ -1877,9 +1877,35 @@ Two smaller consequences of the same asymmetry: ### 7.3 One voice channel per Team +> **Amended 2026-08-19, as built (phase 9).** The org lead settled the access model as **a per-Team +> role, always** — the escalation below is gone, and with it `voice_overwrite_max` and the `mode` +> column. Three things this section names turned out not to exist in the tree at all, and one number +> it relies on counts something different from what it says. Each is marked inline; "as built" wins +> over the original wording wherever they disagree. + **Shape.** One voice channel per qualifying Team, under a single shared parent category (`Teams`), created by the bot. **No per-Team role by default**, no auto-created category per Team. +> **As built: a per-Team role, always.** Overwrites-by-default with escalation was designed to spend +> the scarcer guild-wide resource only where the per-channel budget actually ran out. Roles-always is +> one code path instead of two plus a transition, and it makes the grant a thing a member can be given +> and taken rather than a channel-shaped list — but it moves the ceiling, and that is the part worth +> stating plainly: +> +> | | overwrites (designed) | roles (as built) | +> | --- | --- | --- | +> | Limit | ~100 overwrites **per channel** | 250 roles **per guild** | +> | So the ceiling is | how big ONE Team can be | how many TEAMS can have voice | +> | Visible to other members | no | yes — a role shows on a profile | +> +> A limit on the number of Teams is one an operator has to be told about *before* they reach it, so +> the admin panel reports the guild's role count against the cap and the reconciler refuses the create +> rather than letting Discord reject it. The count comes from the bot, not from core's own rows: the +> cap is shared with every role the operator made themselves. +> +> That a Team's membership becomes visible guild-wide on each member's profile is the trade this +> bought. It is not per-deployment configurable. + **Access, and why overwrites are enough — with a stated fallback.** Access is `@everyone` deny + `VIEW_CHANNEL`/`CONNECT` allow per **linked** Team member (path 4, §2.5) + the staff role. Discord's practical per-channel overwrite budget is ~100. A Team of up to ~95 linked members fits with room for @@ -1889,10 +1915,58 @@ members — because roles are the scarcer guild-wide resource (250 cap) and shou overwrites actually run out. So: overwrites by default, role on demand, and the escalation is recorded in `team_integrations.mode`. +> **As built.** The channel carries exactly three kinds of overwrite: `@everyone` denied, the Team's +> own role allowed, and one allow per operator-designated staff role. Membership is the role's member +> list. There is no `mode`, no `voice_overwrite_max` and no escalation. +> +> **"the staff role" does not exist in this codebase.** `guild_config` knows a news channel, a modlog +> channel, an autorole and a filter allowlist; none of them means "staff", and core has no way to +> derive one. Guild administrators bypass channel overwrites anyway, so what is actually missing is a +> way to let **non-admin** staff in — and only the operator can say which of their roles those are. +> As built: `teams_voice_staff_roles`, a list of role ids, **empty by default and a perfectly ordinary +> answer**. A role the operator has since deleted is filtered out by the bot rather than sent, because +> Discord rejects an entire overwrite set for one bad id and that would take the Team's own grant down +> with it. +> +> **The grant set is hop 3, not path 4's "linked".** A role can only be given to somebody Discord +> knows, so the set is Team members who have a site account *and* a `user_identities` row for Discord +> *and* are in the guild. A member missing the last of those is skipped silently — it is §2.6's hop 3 +> without hop 4, an ordinary state, not an error worth a hundred log lines. + **Provisioning gate.** Admin opt-in per deployment, plus `voice_min_linked_members` (default 5). Counted on **linked** members only, since an unlinked member cannot be granted anything on Discord anyway. +> **As built: `teams_voice_min_members`, counting EVERY active member** (org lead, 2026-08-19). The +> question an operator is answering with this number is "is this Team real enough to deserve a +> channel", and link state answers a different one. Note that this is deliberately *not* +> `teams.linked_count` either — that column counts hop 1 (has a site account), which is a third +> quantity again. +> +> **Two more gates the original does not mention, both required:** +> +> - **A hidden Team is never provisioned.** A channel name is a game-sourced string published outside +> the site, which is exactly §2.8's concern — `utils/reservedNames.js` already names "and eventually +> a Discord channel name" among the surfaces it protects. So the screen that suppresses a Team's +> public page suppresses its channel, and a Team that *becomes* hidden takes the grace window like +> any other removal. The interlock costs one `hidden = 0` in one query rather than a second policy +> that could drift from the first. The name published is `display_name_override || name` — §2.8.3 +> lets staff change what is displayed, and a channel is a display surface. +> - **The bot must actually be able to act.** This section assumes it can manage channels and roles; +> nothing in this project has ever checked. The operator invites the bot by hand and there is no +> invite URL with a permission integer anywhere in the tree, so a deployment can sit one unticked +> box away from every call failing with only a column of identical per-Team errors to show for it. +> As built, a **preflight is a precondition**: `PUT /admin/teams/voice` with `enabled: true` is +> refused **422** while the bot is disconnected or missing Manage Channels or Manage Roles, in the +> same shape §7.2's acknowledgement refuses. It is asked again at the top of every pass. Switching +> voice OFF is never gated — an operator disabling a feature because it is misbehaving must not be +> blocked by the misbehaviour. +> +> The preflight also reports the **bot's own role position**, because that is the second, quieter +> failure: Manage Roles lets the bot create a role, but it can only grant roles *below* its own +> highest. A bot at the bottom of the list creates roles it cannot hand to anybody, which looks exactly +> like a channel nobody can enter. + **Lifecycle: delete, but after a grace window.** Justification, since the brief asks for one: - A voice channel holds **no message history**, so deletion destroys nothing recoverable. The @@ -1908,34 +1982,83 @@ So: drop below threshold → `state='pending_removal'`, `remove_after` = now + ` expiry → delete. A Team **archived** (disbanded or renamed) takes the same window, because "disbanded" can be a missed event and 7 days is cheap insurance. +> **As built, with one narrowing.** "Recover inside the window → **no Discord call made**" is not +> quite what happens, and the truer promise is **no DESTRUCTIVE call**. A Team that climbed back above +> the threshold has members who need granting, and the ordinary membership diff is what grants them; +> refusing to call at all would leave the very people who brought it back outside the channel. What +> the recovery cancels is the deletion, and the channel id is unchanged — which is the whole point. +> +> A **failed teardown keeps the expired window** rather than being rescheduled. Granting another seven +> days each time a delete fails means it never happens. +> +> **Switching voice off tears nothing down.** The pass suspends in both directions and existing +> channels are left standing, inert; the panel says how many remain and offers to remove them one at a +> time. A checkbox must not delete structure in somebody's guild, and an operator trying the feature +> out must be able to stop trying it without consequences. Per-row removal is also the only way to +> clean up while voice is off, since no pass will ever reach those rows. + **And never on stale data.** If `team_sync_state` is stale for the module (§2.4), the integration reconciler **skips entirely** — no creation, no deletion, no overwrite changes. A voice channel is never destroyed because a sidecar was down. +> **As built, and proved on the rig** — a stale projection stops the pass before a single Discord call, +> in both directions, with the row not even scheduled for removal. +> +> One boundary worth knowing: `teams.model.syncStatus()` reports `stale: false` when **no** Team +> provider is registered, on the reasoning that a deployment with no game module is not a broken one. +> So on a deployment whose module has been uninstalled this suspension is inactive — which is benign, +> because with nothing updating the projection the member counts do not move and the reconciler has +> nothing to act on. + ```sql CREATE TABLE IF NOT EXISTS team_integrations ( id INT AUTO_INCREMENT PRIMARY KEY, team_id INT NOT NULL, - platform VARCHAR(32) NOT NULL, + platform VARCHAR(32) NOT NULL, -- 'discord' resource VARCHAR(32) NOT NULL, -- 'voice' external_ref VARCHAR(64) NULL, -- the channel id - mode ENUM('overwrites','role') NOT NULL DEFAULT 'overwrites', - role_ref VARCHAR(64) NULL, + role_ref VARCHAR(64) NULL, -- the Team's role: the grant itself state ENUM('none','active','pending_removal','error') NOT NULL DEFAULT 'none', remove_after DATETIME NULL, last_error VARCHAR(500) NULL, + synced_at DATETIME NULL, updated_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, UNIQUE KEY uq_team_integration (team_id, platform, resource), + INDEX idx_ti_pending (state, remove_after), CONSTRAINT fk_ti_team FOREIGN KEY (team_id) REFERENCES teams(id) ON DELETE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; ``` +> **As built** — `mode` and `role_ref`-as-escalation are gone; `role_ref` is now the grant itself, so a +> row with a channel and no role is a broken row. `synced_at` is added: `updated_at` moves whenever +> core writes a belief, including an error, and "when did this last actually reach Discord" is a +> different question. Unlike §7.2's DDL, this one applied to real MariaDB exactly as written. + **Sync** rides the same reconciliation as membership: after a successful Team reconcile, the integration reconciler diffs the desired access set (path 4) against what the bot reports and issues the minimum set of calls. Every call is best-effort; a failure records `state='error'` with the message and retries on the next pass. It never blocks the Team sync. ---- +> **As built, with the diff on the bot's side.** Core sends the DESIRED STATE for one Team — name, +> category, channel, role, staff roles, the member id list — and the bot works out the calls. That is +> the opposite of the split §7.1 and §7.2 use, and it is deliberate: every *decision* is still core's, +> but the diff is a comparison against live guild state that only the bot can see, and doing it in core +> would mean shipping the guild's whole role membership over the wire to compare it and shipping the +> answer back. +> +> **The membership diff is bounded per pass** (50 operations) and the remainder is reported, because +> each grant is its own API call under its own rate limit and an unbounded first pass on a large guild +> outlives its own request timeout — the one failure that leaves core not knowing what was applied. A +> non-zero remainder asks for another pass rather than waiting out the interval. +> +> **A failure is per-Team and never aborts the pass**, the same shape as §2.4's gate 3. A failed sync +> **keeps the refs it could not confirm**: a failure is core failing to confirm a channel, not learning +> it is gone, and clearing them would orphan a real channel and have the next pass build a second one +> beside it. +> +> The pass is **requested, not awaited**, by the Team reconciler — it makes Discord calls, and a roster +> sync must never be slowed, failed or held open by an integration hanging off it. It has its own +> 30-second debounce. ## Part 8 — Keeping the integration layer platform-agnostic @@ -2709,10 +2832,60 @@ over-long forum post would not have arrived at all rather than arriving truncate **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`) +### Phase 9 — Discord: voice channels (`website`) — **DONE 2026-08-19** -`team_integrations`, the threshold gate, the shared category, overwrite management with role -escalation above `voice_overwrite_max`, the grace-window lifecycle, and the stale-sync suspension. +`team_integrations`, the threshold gate, the shared category, **a per-Team role** (not overwrite +management with escalation — see §7.3's amendment), the grace-window lifecycle, and the stale-sync +suspension. + +**Ships:** every Team above the operator's size threshold gets a voice channel of its own in Discord, +visible and joinable by its members and nobody else. + +**ONE code repo, not the plan's `website` + `bot`.** `bot` is a workspace inside `website` — the same +correction phases 7 and 8 made. `module-uo` is untouched and `MODULE_API_VERSION` does not move. + +**Org-lead decisions (2026-08-19), all four settled before any code:** **roles always**, no overwrite +escalation · the bot creates the parent category and the server stores its id in settings · the +threshold counts **every** active member, not linked ones · "staff" is **a list of Discord roles the +admin designates**, because the concept does not otherwise exist. + +**Walked on the live rig before the PRs opened**, per the order phase 5 set — real MariaDB, the real +app, and a fake standing in for Discord that mounts the bot's real internal routes, so everything up +to the Discord API call was production code. 47 assertions. + +#### What the walk proved, and the two defects it found + +It proved: the preflight refusing an enable three different ways and the panel still rendering with a +broken bot; a category, role and channel created with `@everyone` denied and the Team role allowed; +the hidden Team and the below-threshold Team getting nothing; the role granted to the two members in +the guild and **not** to the one who linked Discord without joining it; a drop below the threshold +scheduling a removal **with zero Discord calls**; a recovery inside the window keeping the same +channel id; an expired window deleting the channel *and* the role and forgetting the row; a stale +projection suspending the pass in both directions; voice switched off leaving the channels standing; +and an admin removal working anyway, with a 404 for a Team that has none. + +1. **Every query failed on a duplicate result column.** `desiredTeams` and `holdersWithoutClaim` both + select `t.id AS team_id`, and the shared column list added `i.team_id` beside it — which the + `mariadb` driver refuses outright ("Error in results, duplicate field name `team_id`"). The pass + died at its first query, on the one code path every unit test stubs. It was also the wrong column: + `desiredTeams` LEFT JOINs, so `i.team_id` is NULL for exactly the Teams that have no channel yet. +2. **"Sync now" reported "Nothing was done" while it was doing it.** Saving the settings with voice on + asks for a pass; an operator pressing Sync now next — the obvious thing — got "a pass is already + running" and a panel saying nothing had happened, while the pass they triggered created their + channels. A pass in flight is now joined and its real outcome returned, as `reconcileNow` does. + +#### Two things outside this phase that it had to work around + +- **`npm run swagger` could not run at all on `edge`.** Phase 8 shipped a regex literal followed + directly by `.test(` in a route validator, which makes swagger-autogen's parser run away and the + process die out of memory. Hoisted to a const. Underneath it, `teams.router.js` sits exactly at that + parser's **per-file limit**: at twenty `teamsRouter.*` statements it dies and at nineteen it + generates, and one more statement of any shape tips it — an unannotated route does, and so does a + bare `use`. The voice routes are therefore their own router file, mounted from `admin/index.js`. +- **`last_success_at` is written by MariaDB's `NOW()` and compared against JS `Date.now()`**, so an app + process and a database in different timezones skew every staleness judgement by the offset — which + moves §3's public freshness banner as much as this phase's suspension. Pre-existing and not fixed + here; recorded because it is invisible until something depends on it. ### Phase 10 — the capability layer (`website` + `docs`)