From aed4ec41668ecd23d4f1758b443a93a95bb3a49d Mon Sep 17 00:00:00 2001 From: wtclaude Date: Tue, 18 Aug 2026 13:29:37 -0500 Subject: [PATCH] =?UTF-8?q?docs(teams):=20phase=205=20=E2=80=94=20discussi?= =?UTF-8?q?on,=20the=20edit=20window,=20and=20reports=20that=20route=20aro?= =?UTF-8?q?und=20leadership?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TEAMS.md §5.4, §5.5.7 (new), §5.6 and the Part 12 phase entry; BACKEND_DESIGN.md's schema and admin route tables. **The largest change is a decision, not a description.** §5.6's first rule said "a leader may also see and act on reports for their own Team, but staff always receive them". The org lead settled on 2026-08-18 that the leader half is **decided against, not deferred**: the gap the section exists to close is that leaders moderate their own forum and a Team's leaders are exactly the people who will not report their own Team, so a leader-visible queue hands a complaint about a leader back to them — and a read-only leader view still tells them who reported what. Recorded as an amendment rather than by editing the sentence away, because the reasoning for the original is what makes the correction legible. **§5.6's `content_reports` DDL does not work as written, and the amendment says so rather than quietly swapping it.** With `status` in the unique key, CLOSED rows collide with each other: dismiss a report, let the behaviour recur, dismiss the second one, and the UPDATE lands on a tuple that already exists — so the queue starts throwing duplicate-key errors on the first repeat reporter. The shipped table keys on a generated `open_marker`, the same encoding `team_forum_grants.active_marker` uses. Three smaller departures are recorded beside it: `handled_note`, the two username snapshots §2.10 asks for everywhere else, and a real CASCADE on `team_id`. **New §5.5.7 for `teams_forum_edit_window_minutes`** (0–1440, default 15), and the rule under it: the window is resolved on the server TWICE — the read path stamps `canEdit`/`editableUntil` so a client knows whether to draw the control, the write re-derives it from `created_at` before allowing anything. The read is advice and the write is enforcement, because a time-bounded permission must not take its clock from the party it bounds. That is also why the key is not published: the client needing the number is the admin screen, and the client needing the decision already has it per post. **§5.4 gains three notes its route table does not carry**: thread creation splits authority by TYPE rather than widening the leader gate (and reports it as two booleans, since one would make a client guess which right it described); post moderation is its own route whose validator accepts all eight actions so the model can say "pin applies to a thread, not to a post"; and a reply's three refusal codes are chosen to be distinguishable — 404 absent, 400 announcement, 409 locked — with locked refusing staff too. The Part 12 entry records what the phase disproved, its four acceptance criteria, that it spans ONE repo where phase 4 needed two, and the single defect the live rig found. It also notes that phase 4 shipped `uploads` with the default off, so §5.6's "pull reports forward if uploads is enabled anywhere" never triggered. Co-Authored-By: Claude --- website/BACKEND_DESIGN.md | 39 +++++++++- website/TEAMS.md | 158 +++++++++++++++++++++++++++++++++++++- 2 files changed, 192 insertions(+), 5 deletions(-) diff --git a/website/BACKEND_DESIGN.md b/website/BACKEND_DESIGN.md index b436b29..33fa4c4 100644 --- a/website/BACKEND_DESIGN.md +++ b/website/BACKEND_DESIGN.md @@ -525,7 +525,12 @@ have been found earlier, because until then no caller had ever passed a non-null Design of record: [`MODULE_SYSTEM.md`](MODULE_SYSTEM.md) §2.4; the loader's obligations are [`MODULE_API.md`](MODULE_API.md) Part 4. -### The eleven Team tables — core's, populated by a module (Teams phases 2–4) +### The eleven Team tables — core's, populated by a module (Teams phases 2–5) + +*Twelve rows in the table below: `content_reports` is listed here because Team forum content is its +first consumer, and it is deliberately **not** one of the eleven — it carries no `team_*` prefix, its +`target_type` is an open VARCHAR, and a wiki page or a news comment is meant to become a value in it +rather than a table of its own.* A Team is a **core** entity that a **module** answers for. The module says what Teams exist and who is in them, through the team provider; core stores that answer, gates it and displays it. Every table @@ -547,6 +552,37 @@ core's. | `team_forum_posts` | post bodies, sanitised on write through the forum's **own** profile (`utils/forumHtml.js`) and served without re-sanitising. No stored body ever contains an `` | | `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 | +| `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`, +`mod_notes` and `appeals` are all either staff-initiated or Discord-sanction-shaped; nothing anywhere +let a *member* say "this is a problem". That was survivable while every piece of content on the site +came from staff, and stops being the moment a Team forum lets players write to each other. Four +properties are worth carrying: + +- **Reports reach site staff and nobody else.** A Team's leaders moderate their own forum, so a + leader-visible queue would route a complaint *about* a leader back to that leader. There is one + queue, mounted at `/admin/moderation/reports` beside appeals — a staffer working a queue should have + one place to work — and no leader-facing counterpart anywhere + ([`TEAMS.md`](TEAMS.md) §5.6, org lead 2026-08-18). +- **A report is not a moderation action.** Filing one changes nothing about the content; it opens a + queue item. That keeps it clear of `team_forum_moderation`, which records things that actually + happened, and stops "report" becoming a way for any participant to hide anything. +- **One OPEN report per (target, reporter)**, enforced by a unique key over a generated `open_marker` + that is `1` while open and `NULL` once closed — the same encoding as + `team_forum_grants.active_marker`, and for the same reason: only the *live* rows may collide. A + closed report frees the slot, so a member whose first report was dismissed may raise the same target + again if the behaviour recurs. +- **Every transition writes `activity_log`, `dismissed` included.** A queue where acting is audited and + declining to act is not is one where the cheapest way to make a report vanish leaves no trace. + +**`teams_forum_edit_window_minutes`** (0–1440, default 15) bounds how long an author may edit their own +post; staff are not bound by it. It is resolved on the server **twice** — the read path stamps each +post with `canEdit`/`editableUntil` so a client knows whether to draw the control, and the write +re-derives it from `created_at` before allowing anything. The read is advice, the write is enforcement, +and the split exists because a time-bounded permission must not take its clock from the party it +bounds. It is deliberately **not** in `settings.getPublic()`: the client that needs the number is the +admin screen, and the client that needs the decision already has it per post. **The forum's tables are guarded at the ROUTE and never at the data.** `teams_forums_enabled` off means every forum route answers **404** — not 403, which would advertise a feature the operator @@ -960,6 +996,7 @@ file a route sits in — that is the property the route manifest freezes. | POST | `/teams/:id/unhide` · `/teams/:id/display-name` | the two **gated** actions (§2.9): an admin applies at once, a **moderator** files a pending request and nothing changes publicly. The caller does not choose — the server decides from the role it re-validates on the request | | GET | `/teams/:id/grants` | the full forum-grant ledger, revoked rows included. Read-only in this phase; the grant flow lands with the forums | | POST | `/teams/:id/leader-override` · DELETE `…/:memberKey` | set or clear a staff leadership decision, applied **on top of** the synced value at read time. Not gated: it publishes no game-sourced string | +| GET | `/moderation/reports` · POST `…/:id/handle` | the member-raised content-report queue (phase 5, [`TEAMS.md`](TEAMS.md) §5.6) and the staff decision on one. Mounted under **moderation**, not under Teams: a staffer working a queue should have one place to work, and `target_type` is open-ended so the next reportable thing arrives as a row rather than as a screen. Each row carries its target already resolved — a post's excerpt and author, a thread's title, or an upload's uploader, byte size and **sniffed** mimetype — in three batched reads, never one per row. A target hard-deleted since reporting comes back `null` and the row still lists. **There is no leader-facing counterpart to either route**, deliberately | | GET | `/teams/review` | the reserved-name review queue — Teams auto-hidden because their name matched, each showing which term | | GET | `/teams/requests` · POST `…/:id/decide` | the approval queue, and the decision. **Admin only** to decide, checked live rather than from a token claim; a request already decided returns `409`, so two admins deciding at once cannot double-apply | | — | `/shard/*` · `/uo-link/*` | **Served by `module-uo`, not by core** (33 routes). Documented in [`../modules/uo/API.md`](../modules/uo/API.md) | diff --git a/website/TEAMS.md b/website/TEAMS.md index b7925c8..58800f3 100644 --- a/website/TEAMS.md +++ b/website/TEAMS.md @@ -1071,6 +1071,31 @@ reason (§5.5.1). > toggle-off revokes no grant and the rows stay authoritative (§5.5.1), so the access list has to stay > manageable during one. What the switch guards is the forum's CONTENT. +> **Amended 2026-08-18 (phase 5).** The 5b routes are as tabled, with three notes the table does not +> carry. +> +> **`POST /forum/threads` splits its authority BY TYPE rather than widening the leader gate.** An +> `announcement` stays leader-authored; a `discussion` may be opened by any forum participant — +> including a granted non-member with no game identity, which is path 3 doing its job. `type` defaults +> to `announcement`, so a phase-4 client keeps meaning what it meant; defaulting the other way would +> silently turn its announcements into discussions. The list response reports the split as **two** +> booleans, `canPost` (may open a discussion) and `canAnnounce` (leader), because a client reading one +> boolean would have to guess which right it described. +> +> **Post-level moderation is its own route**, `POST /forum/posts/:id/moderate`, rather than the thread +> route with a target kind: `pin` and `lock` describe a thread's place in a list and its openness to +> replies, neither of which a post has. The route's validator deliberately accepts **all eight** +> actions so the model can answer `pin` with *"pin applies to a thread, not to a post"* — restricting +> it to the four a post takes turns a nameable mistake into a generic validation error, which is what +> the live rig found. +> +> **Three refusal codes on a reply, chosen to be distinguishable.** 404 for a thread that is absent or +> hidden from this caller; **400** for an announcement, which takes no replies by TYPE and no retry +> fixes; **409** for a locked thread, where the request is well-formed and the resource's state is what +> refuses. Locked refuses **staff too** — they hold `unlock`, so unlock/post/relock reaches the same +> place leaving three ledger rows that say what happened, whereas a moderator's reply in a thread +> nobody else may answer is the last word by fiat. + Under `/player` for the same reason as §2.11: a forum participant may be a plain player, and the tier gate is `requireAuth`. Every route resolves access through the §2.5 resolver — never by checking membership directly, which is how paths 1 and 3 would drift back together. @@ -1300,6 +1325,34 @@ who accepted a liability notice is operator detail, exactly as `failure_reason` The *rendering* decision is still made server-side. The client is told the mode so it can present the right composer; it is never the thing that decides whether an image appears. +#### 5.5.7 `teams_forum_edit_window_minutes` — how long an author may edit (phase 5) + +An ordinary `settings` key, `0`–`1440`, **default 15**, on the same admin screen as the other two. Set +to `0` it makes posts permanent once written, which is a legitimate operator choice rather than an +off switch — there is no state in which editing is "disabled" as opposed to "bounded at zero", and +inventing one would only give the resolver a decision to get wrong. + +**Staff are not bound by it.** The window exists so a post cannot be rewritten out from under someone +quoting it, or under a moderator about to act on a report; a staffer editing another member's post is +already an intervention that writes `activity_log` (§5.3), and time-bounding it would only mean +waiting. + +**It is evaluated on the server twice, on purpose.** The read path stamps every post with `canEdit` +and `editableUntil` so a client knows whether to draw the control; the write re-derives it from +`created_at` before allowing anything. Two evaluations of one rule: the read one is advice and the +write one is enforcement. A client may use `editableUntil` to WITHDRAW an offer whose deadline passed +while a page sat open, and can never create one — **a time-bounded permission must not take its clock +from the party it bounds**, which is why the window itself is not a published setting (§5.5.6) and is +served only to the admin screen that edits it. + +A hidden or deleted post is editable by nobody, staff included. Restoring it is a moderation action +with a ledger row; quietly rewriting it while it is out of sight is the same act with no record. + +The read fails closed to **zero**, not to the default — the opposite of what it looks like it should +do. The risk the window bounds is an author rewriting a post out from under a reader, so the safe +answer during a DB fault is "nobody may edit for the next minute". A stale uploads acknowledgement +freezes this key along with the other two: it is a forum setting. + ### 5.6 Abuse reports — the missing half of moderation **Core has no user-facing report flow of any kind today.** `moderation`, `mod_notes` and `appeals` are @@ -1335,11 +1388,46 @@ CREATE TABLE IF NOT EXISTS content_reports ( ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; ``` +> **Amended 2026-08-18 (phase 5). The table as shipped departs from the block above in four places, +> three of them corrections and one an addition.** +> +> **The unique key is on a generated `open_marker`, not on `status`, and the spelling above has a +> defect worth recording rather than quietly fixing.** With `status` in the key, CLOSED rows collide +> with each other too: a reporter reports a post, staff dismiss it, the behaviour recurs, they report +> it again — and the second dismissal is an `UPDATE` into a `(…, 'dismissed')` tuple that already +> exists, so working the queue starts throwing duplicate-key errors on the first repeat reporter. The +> shipped column is `open_marker TINYINT(1) AS (IF(status IN ('open','reviewing'), 1, NULL)) STORED`, +> the same trick `team_forum_grants.active_marker` uses: 1 while open, NULL once closed, and MySQL +> treats NULLs as distinct — so any number of closed reports coexist while at most one open one can. +> That is what the prose above actually asks for. +> +> **`handled_note VARCHAR(500)`** was added. §5.6's API takes `{ status, note? }` and the table had +> nowhere to put the note. A queue whose resolution reason lives only in an `activity_log` line is one +> where the next staffer to see a repeat report about the same content cannot find out why the last +> one was closed. +> +> **`reporter_username` and `handled_username` snapshots** were added, per §2.10: who raised a report +> and who decided it must survive the account, exactly as every other Team table already does. +> +> **`team_id` gained a real FK with `ON DELETE CASCADE`.** The block above leaves it a bare +> denormalised column; a deleted Team then leaves a queue full of reports about content that cascaded +> away with it. +> +> Every transition writes `activity_log`, **`dismissed` included**. A queue where acting is audited and +> declining to act is not is one where the cheapest way to make a report vanish leaves no trace — and +> the reports most worth auditing are exactly the ones somebody wanted gone. + Four rules: -- **Reports go to site staff, not to Team leaders.** A leader may also see and act on reports for - their own Team, but staff always receive them — the whole point is a path that routes *around* a - Team's own leadership. +- **Reports go to site staff, and to nobody else.** *(Amended 2026-08-18, org lead, when phase 5 was + built.)* This section originally added "a leader may also see and act on reports for their own + Team". **That half is not implemented and is not deferred — it is decided against.** The gap this + whole section exists to close is that leaders moderate their own Team's forum and a Team's leaders + are exactly the people who will not report their own Team; a leader-visible queue hands a complaint + *about* a leader straight back to them, and a read-only leader view still tells them who reported + what. There is one queue, under `/admin/moderation`, gated to admin + moderator. If a leader-facing + surface is ever wanted it is a fresh design decision, not a refactor — `content_reports.team_id` + makes it *possible*, which is not the same as intended. - **Reporting is not a moderation action.** A report changes nothing about the content; it opens a queue item. This keeps it clear of §5.3's leader/staff moderation ledger, which records things that actually happened. @@ -1355,6 +1443,20 @@ GET /api/v1/admin/moderation/reports the queue, alongside the exis POST /api/v1/admin/moderation/reports/:id/handle { status, note? } ``` +The player route sits behind the same `resolveForum` guard as the rest of §5.4, so a reporter is by +construction someone who can already see what they are reporting — and the model additionally checks +the target really belongs to the Team the request came through, or the queue's per-Team filter would +quietly be lying. A duplicate answers **409** rather than pretending to succeed: silently accepting is +friendlier for one tap and dishonest for the second, and a member who reports twice because nothing +seemed to happen deserves to be told the first is already in the queue. + +The queue resolves every row's target in **three batched reads** keyed by target type, never one read +per row — that is rule 4 above actually paying for §5.5.4's attribution table, and the N+1 version is +how a queue becomes a thing staff avoid opening. A target that has since been hard-deleted comes back +as `null` and the report still lists: "somebody reported this and by the time we looked it was gone" +is a fact a moderator needs, and dropping the row would hide the pattern of a member deleting their +own content the moment it is reported. + Mounted under the **existing** admin moderation section rather than under Teams: a staffer working a queue should have one place to work, and a report about a forum post is the same job as a report about anything else. @@ -2231,6 +2333,34 @@ to run it, at low surface area. ### Phase 5 — Forum 5b: discussion + moderation + reports (`website`) +> **Amended 2026-08-18, while building this.** Five notes. The first is the org lead's decision; the +> rest were found by building the thing described below, or on the live rig afterwards. +> +> **Reports are site administration only.** §5.6's "a leader may also see and act on reports for their +> own Team" is decided against, not deferred — see the amendment there. It is the phase's most +> important property and it is a NEGATIVE one, so it is asserted directly in the test suite rather +> than left to be noticed: the report model's whole function surface is pinned, and `queue`/`handle` +> are checked not to mention leadership at all. +> +> **The edit window is an admin setting, not a constant** (§5.5.7), and it is evaluated on the server +> twice — once as advice on the read path, once as enforcement on the write. That is the phase's other +> structural rule: a time-bounded permission must not take its clock from the party it bounds. +> +> **§5.6's unique key does not work as written**, and the shipped table uses a generated `open_marker` +> instead. See the amendment there; it is the one place in this document where the SQL and the prose +> beside it disagreed. +> +> **This phase spans ONE repo, which is worth saying because phase 4 did not.** Phase 4 needed +> `module-uo` because the forum had no surface after phase 3 and a slot had to be declared. Phase 5 +> grows the component that fills that slot, so `uo.guild.forum` is untouched and nothing in the module +> changes. +> +> **The live rig found one defect, and it was a message rather than a behaviour.** The post-moderation +> route's validator listed only the four actions a post accepts, so `pin` returned a generic +> "Validation failed" instead of the sentence written for it — leaving that branch reachable only from +> its own unit test. Walking the surface for real is what turns "documented, tested and unreachable" +> into something anyone notices. + Discussion threads, replies, the edit window, pin/lock/hide/delete, `team_forum_moderation`, the admin ledger view, and **abuse reporting** (§5.6): `content_reports`, the report control, and the queue in the existing admin moderation section. @@ -2238,7 +2368,27 @@ the existing admin moderation section. Reports land here rather than in Phase 4 only because discussion is what generates them at volume — if Phase 4 ships `uploads` mode enabled anywhere before Phase 5, **pull reports forward into Phase 4**. An upload path with a liability acknowledgement and no way for a member to raise a problem is the one -combination this plan should not ship. +combination this plan should not ship. *(In the event, phase 4 shipped `uploads` mode with the default +off, so nothing was pulled forward.)* + +Also lands here, because both had existed since phase 4 with nothing rendering them: the **per-Team +forum moderation ledger** on the admin Teams screen — the `actor_role` column that keeps a leader's +housekeeping distinguishable from a staff intervention was readable only from a DB client — and +`softDeleteUploadsForPost`, which post deletion is the first caller of and which needed an inverse so +`delete` → `restore` does not return a post's words while silently losing its pictures a retention +window later. + +**Acceptance, four:** +1. A member opens a discussion and a granted non-member replies to it; the same member is refused an + announcement `403` while a leader is allowed one. +2. A locked thread refuses replies at `409` from every identity **including staff**, and unlock → + reply → relock leaves three rows in the Team's ledger saying so. +3. An author edits their own post inside the window and is refused `403` outside it; staff edit the + same post at any time, and a staff edit of somebody else's post writes `activity_log` while a + member's own edit does not. +4. A member reports a post; the report reaches `/admin/moderation/reports` and answers `403` to the + Team's own leader, to the reporting member and to every other participant; handling it changes the + report's status and **nothing at all** about the content. ### Phase 6 — Team notifications (`website`) -- 2.49.1