feat(teams): phase 5 — Forum 5b, discussion + moderation + reports #155

Merged
whitlocktech merged 5 commits from feature/teams-phase5-discussion into edge 2026-08-18 18:36:52 +00:00
Member

Teams Phase 5 (TEAMS.md §5.1's "5b"), into edge. Docs half: RunicGateway/docs#158.

One repo, where phase 4 needed two. Phase 4 spanned 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 uo.guild.forum, so the module is untouched.

No migration. Every column this needed — type, locked, edited_at, edited_by, the post table's status, the ledger's target_type='post' — landed in phase 4. That was the point of §5.1's split-by-layer. The only new table is content_reports.


What it ships

Discussion threads authority splits by type: announcements stay leader-authored, discussions open to any participant — including a granted non-member with no game identity
Replies 404 absent · 400 announcement (takes none by TYPE) · 409 locked (well-formed request, refusing state)
Edit window teams_forum_edit_window_minutes, 0–1440, default 15; staff unbounded
Post moderation hide/unhide/delete/restore, same ledger via target_type='post'
Abuse reports content_reports, the report control, and the staff queue
Admin ledger the per-Team forum log — route + API method existed since phase 4 with nothing rendering them

Three properties worth reviewing directly

1. Reports reach site staff and nobody else. §5.6 originally added "a leader may also see and act on reports for their own Team". Org lead settled 2026-08-18 that this is decided against, not deferred — the gap the feature closes is that leaders moderate their own forum and a Team's leaders are exactly the people who will not report their own Team. It is a negative property, and negatives are what nobody notices going, so it is asserted directly: contentReports.test.js pins the model's whole function surface and checks queue/handle never mention leadership.

2. The edit window is resolved on the server twice. The read path stamps each post with 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, the write is enforcement — a time-bounded permission must not take its clock from the party it bounds. The client may only ever withdraw an offer whose deadline passed while a page sat open.

3. A locked thread refuses staff too. They hold unlock, so nothing is lost — and locked then means the same thing to every reader. Unlock/post/relock reaches the same place leaving three ledger rows that say what happened; a moderator's reply in a thread nobody else may answer is the last word by fiat.

§5.6's DDL does not work as written

With status in the unique key, closed rows collide with each other: dismiss a report, let the behaviour recur, dismiss the second — and the UPDATE lands on a (…, 'dismissed') tuple that already exists, so the queue throws duplicate-key errors on the first repeat reporter. The table keys on a generated open_marker instead (1 while open, NULL once closed), the same encoding team_forum_grants.active_marker uses. Three smaller departures are recorded in the docs PR: handled_note, the two §2.10 username snapshots, and a real CASCADE on team_id.

Tests — 1009 server (was 972), 241 client (was 224)

Four acceptance criteria, plus the negative property above. The suite caught one real defect while being written: describeTarget returned undefined for a hard-deleted target, and undefined is dropped by JSON.stringify, so the documented target: null would have reached clients as an absent key.

Walked on the live rig before opening this

Per the org lead's call. Real server, real MariaDB, real sessions, four identities — ordinary member, granted non-member guest, Team leader, staffer — plus a browser pass over the forum panel, the reports queue, the per-Team ledger and the settings screen. Confirmed live: a locked thread refuses all four at 409; a hidden post renders for leader and staff with Unhide and no Edit control for anyone; the report queue answers 200 to staff and 403 to the leader, the member and the guest alike; the edit window at 0 stops the author while leaving staff unbounded.

It found one defect (last commit): the post-moderation validator listed only the four actions a post accepts, so pin returned a generic "Validation failed" instead of "pin applies to a thread, not to a post" — leaving that branch reachable only from its own unit test.

Two phase-4 fixes this work reached

softDeleteUploadsForPost bound its arguments in the wrong order (never fired — nothing called it until post deletion did), and had no inverse, so deleterestore would have returned a post's words and silently lost its pictures a retention window later.


Route manifest and OpenAPI regenerated: 6 operations added, 0 lost (checked explicitly — that is the silent-deletion failure mode slotSpecs.js warns about).

  • AI-assisted — Claude Code (Claude Opus 5)

🤖 Generated with Claude Code

https://claude.ai/code/session_01WnDSWzpUjw8t8C2hghysNz

Teams **Phase 5** (`TEAMS.md` §5.1's "5b"), into `edge`. Docs half: RunicGateway/docs#158. **One repo, where phase 4 needed two.** Phase 4 spanned `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 `uo.guild.forum`, so the module is untouched. **No migration.** Every column this needed — `type`, `locked`, `edited_at`, `edited_by`, the post table's `status`, the ledger's `target_type='post'` — landed in phase 4. That was the point of §5.1's split-by-layer. The only new table is `content_reports`. --- ### What it ships | | | |---|---| | Discussion threads | authority splits **by type**: announcements stay leader-authored, discussions open to any participant — including a granted non-member with no game identity | | Replies | 404 absent · **400** announcement (takes none by TYPE) · **409** locked (well-formed request, refusing state) | | Edit window | `teams_forum_edit_window_minutes`, 0–1440, default 15; staff unbounded | | Post moderation | hide/unhide/delete/restore, same ledger via `target_type='post'` | | Abuse reports | `content_reports`, the report control, and the staff queue | | Admin ledger | the per-Team forum log — route + API method existed since phase 4 with **nothing rendering them** | ### Three properties worth reviewing directly **1. Reports reach site staff and nobody else.** §5.6 originally added "a leader may also see and act on reports for their own Team". Org lead settled 2026-08-18 that this is **decided against, not deferred** — the gap the feature closes is that leaders moderate their own forum and a Team's leaders are exactly the people who will not report their own Team. It is a *negative* property, and negatives are what nobody notices going, so it is asserted directly: `contentReports.test.js` pins the model's whole function surface and checks `queue`/`handle` never mention leadership. **2. The edit window is resolved on the server twice.** The read path stamps each post with `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, the write is enforcement — **a time-bounded permission must not take its clock from the party it bounds.** The client may only ever *withdraw* an offer whose deadline passed while a page sat open. **3. A locked thread refuses staff too.** They hold `unlock`, so nothing is lost — and `locked` then means the same thing to every reader. Unlock/post/relock reaches the same place leaving three ledger rows that say what happened; a moderator's reply in a thread nobody else may answer is the last word by fiat. ### §5.6's DDL does not work as written With `status` in the unique key, **closed rows collide with each other**: dismiss a report, let the behaviour recur, dismiss the second — and the `UPDATE` lands on a `(…, 'dismissed')` tuple that already exists, so the queue throws duplicate-key errors on the first repeat reporter. The table keys on a generated `open_marker` instead (1 while open, NULL once closed), the same encoding `team_forum_grants.active_marker` uses. Three smaller departures are recorded in the docs PR: `handled_note`, the two §2.10 username snapshots, and a real CASCADE on `team_id`. ### Tests — 1009 server (was 972), 241 client (was 224) Four acceptance criteria, plus the negative property above. The suite caught one real defect while being written: `describeTarget` returned `undefined` for a hard-deleted target, and `undefined` is dropped by `JSON.stringify`, so the documented `target: null` would have reached clients as an absent key. ### Walked on the live rig **before** opening this Per the org lead's call. Real server, real MariaDB, real sessions, four identities — ordinary member, granted non-member guest, Team leader, staffer — plus a browser pass over the forum panel, the reports queue, the per-Team ledger and the settings screen. Confirmed live: a locked thread refuses all four at 409; a hidden post renders for leader and staff with **Unhide** and **no Edit control for anyone**; the report queue answers 200 to staff and **403 to the leader**, the member and the guest alike; the edit window at 0 stops the author while leaving staff unbounded. **It found one defect** (last commit): the post-moderation validator listed only the four actions a post accepts, so `pin` returned a generic "Validation failed" instead of *"pin applies to a thread, not to a post"* — leaving that branch reachable only from its own unit test. ### Two phase-4 fixes this work reached `softDeleteUploadsForPost` bound its arguments in the wrong order (never fired — nothing called it until post deletion did), and had no inverse, so `delete` → `restore` would have returned a post's words and silently lost its pictures a retention window later. --- Route manifest and OpenAPI regenerated: **6 operations added, 0 lost** (checked explicitly — that is the silent-deletion failure mode `slotSpecs.js` warns about). - [x] AI-assisted — Claude Code (Claude Opus 5) 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01WnDSWzpUjw8t8C2hghysNz
wtclaude added 5 commits 2026-08-18 18:31:08 +00:00
Phase 5's server half — TEAMS.md §5.1's "5b". The schema for all of it landed in
phase 4, so this adds no ALTER: every column it needed (`type`, `locked`,
`edited_at`, `edited_by`, the post table's `status`, the ledger's
`target_type='post'`) was already there waiting.

  * `teams_forum_edit_window_minutes` (0…1440, default 15) joins the forum's
    settings. It fails closed to ZERO rather than to its default, which is the
    opposite of what it looks like it should do: the risk an edit window bounds is
    an author rewriting a post out from under a reader quoting it or a moderator
    about to act on a report, so the safe answer during a DB fault is "nobody may
    edit for the next minute". A stale uploads acknowledgement freezes this key
    too — it is a forum setting.

  * Thread creation splits its authority BY TYPE, which is what phase 4's comment
    said would happen here rather than widening the leader gate. An announcement
    stays leader-authored; a discussion is open to every participant, and
    "participant" includes a granted non-member with no game identity — path 3
    doing its job. `type` still defaults to `announcement`, so a phase-4 client
    keeps meaning what it meant.

  * Replies refuse three ways with deliberately different codes: 404 for absent or
    hidden, 400 for an announcement (which takes no replies by TYPE, not by being
    closed), and 409 for locked — well-formed request, refusing state. Locked
    refuses staff too; they hold `unlock`, and unlock/post/relock reaches the same
    place leaving three ledger rows that say so.

  * The edit window is evaluated on the server twice, on purpose. The read path
    stamps every post with `canEdit`/`editableUntil` so the client knows whether to
    draw the control; the write re-derives it from `created_at` before allowing
    anything. A time-bounded permission must not take its clock from the party it
    bounds. Staff are not time-bounded, and a staff edit of someone else's words
    writes `activity_log` while a member fixing their own typo does not (§5.3).

  * Post moderation shares the thread ledger via `target_type='post'`, so
    "everything moderated in this Team" stays one query. `pin`/`lock` are refused
    by name rather than as unknown actions — they describe a thread's place in a
    list and its openness to replies, neither of which a post has. Counters are
    RECOMPUTED after each action rather than nudged, because hide → unhide → hide
    is a cycle a delta gets wrong the first time a step is retried.

Two fixes to phase 4 code this work reached: `softDeleteUploadsForPost` bound its
two arguments in the wrong order (never fired — nothing called it until post
deletion did), and it had no inverse, so `delete` → `restore` would have returned
a post's words and silently lost its pictures a retention window later.

Co-Authored-By: Claude <noreply@anthropic.com>
TEAMS.md §5.6. **Core has had no user-facing report flow of any kind** — the
`moderation`, `mod_notes` and `appeals` tables are all either staff-initiated or
Discord-sanction-shaped, and nothing anywhere let a member say "this is a
problem". That was survivable while every piece of content on the site came from
staff; phase 5 lets players write to each other, so it stops being.

The gap has a specific shape: leaders moderate their own Team's forum, and a
Team's leaders are exactly the people who will not report their own Team. So the
whole point of this queue is a path that routes AROUND a Team's own leadership.
Org lead settled it on 2026-08-18: **reports are site administration only** —
there is no leader-facing view of this queue, not even a read-only one scoped to
their own Team. §5.6's "a leader may also see and act on reports for their own
Team" is not implemented and is not deferred.

`content_reports` is deliberately generic — `target_type` is a VARCHAR so a wiki
page or a news comment becomes a value rather than a table — and the queue is
mounted beside appeals under /admin/moderation rather than under Teams, because a
staffer working a queue should have one place to work.

**§5.6's literal unique key has a defect and this does not copy it.** Written as
(target_type, target_id, reporter_user_id, status) it makes CLOSED rows collide
with each other too: reporter reports a post, staff dismiss it, the behaviour
recurs, they report again — and the second dismissal is an UPDATE into a tuple
that already exists, so working the queue starts throwing duplicate-key errors on
the first repeat reporter. The key is on a generated `open_marker` instead, the
same trick `team_forum_grants.active_marker` uses: 1 while open, NULL once
closed, and NULLs are distinct — which is what §5.6's prose asks for, "one open
report per (target, reporter)".

Two other departures from the doc, both small and both flagged in the docs PR:
`handled_note`, because a queue whose resolution reason lives only in an
activity_log line is one where the next staffer to see a repeat report cannot
find out why the last was dismissed; and a CASCADE on `team_id`, so a deleted
Team does not leave a queue full of reports about content that no longer exists.

Also here: a report is filed against a target the model verifies really belongs to
the Team the request came through, or the queue's per-Team filter would quietly be
lying; the queue resolves every row's target in three batched reads rather than
N+1, which is §5.6's fourth rule (uploader, size and sniffed type without
hunting) actually paying for §5.5.4's attribution table; a target that has since
been hard-deleted comes back null and the report still lists, because "somebody
reported this and by the time we looked it was gone" is a fact a moderator needs;
and 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` gains its range validation on the admin settings
PUT and is seeded at 15, so the value on the settings screen is the value in
force. Route manifest and OpenAPI regenerated: 6 operations added, 0 lost.

Co-Authored-By: Claude <noreply@anthropic.com>
1008 pass (972 before). The tests worth reading first are the ones that pin a
property no screen would look different without:

  * **The edit window is decided on the server, twice.** One test proves the read
    path stamps `canEdit` per post per viewer; another proves the WRITE path
    re-derives it from `created_at` and refuses a stale edit even though the
    client was told it could — because a time-bounded permission must not take its
    clock from the party it bounds.

  * **A locked thread refuses staff too**, asserted over member, leader and staff
    in one loop, at 409 rather than 403: well-formed request, refusing state.

  * **delete → restore is reversible for images.** Without the second half of the
    pair a restored post returns its words and loses its pictures a retention
    window later, silently — the test asserts both calls and that `hide` makes
    neither.

  * **Post moderation recomputes the thread's counters** rather than nudging them;
    the test runs hide → unhide → hide, which is the cycle a delta gets wrong.

  * **acceptance: nothing in the report model is reachable by a Team leader.** The
    negative property is the whole point of §5.6 and negatives are what nobody
    notices going, so it is asserted directly — the module's function surface is
    pinned, and `queue`/`handle` are checked not to mention leadership at all. If
    a leader-facing queue is ever wanted it is the org lead's decision, and this
    test is what makes somebody ask.

  * **A report never changes the content it is about**, proved by stubbing every
    mutation the forum has to throw. If filing a report touched a status then
    "report" would BE moderation, and the first person to work that out would have
    found a way to hide anything on the site.

The test suite caught one real defect: `describeTarget` returned `undefined` for a
hard-deleted target, and `undefined` is dropped by JSON.stringify — so the
documented `target: null` would have reached clients as an absent key.

Two phase-4 tests were updated rather than added to, both because phase 5 changed
what they describe: `canPost` split into `canPost` (open a discussion, everyone)
and `canAnnounce` (leaders), and `discussion` is no longer a refused thread type.
Phase 5's four new player routes are added to acceptance criterion 2's list, so
"with the forum off every forum route 404s" keeps covering the whole surface.

Co-Authored-By: Claude <noreply@anthropic.com>
241 client tests pass (224 before).

**The forum panel becomes a forum.** It was "Announcements" with one composer;
it now has two, because phase 5 split one server capability into two: `canPost`
means "may open a discussion" and every participant may — a granted guest with no
game character included, which is path 3 doing its job — while `canAnnounce` is
the leader-only half `canPost` used to carry alone. Threads gain replies, an edit
control, per-post moderation and a report control, all still inside the one slot
the module declares, still navigating by `?thread=`.

**Almost nothing here is the client's decision, and the file says so.** `canPost`,
`canAnnounce`, `canReply` and each post's `canEdit`/`editableUntil` are read, not
computed. The one local judgement is a ticking clock that WITHDRAWS an edit offer
whose deadline passed while the page sat open — it can never grant one, because a
time-bounded permission must not take its clock from the party it bounds. That
asymmetry is the first thing client/test/teamForum.test.js asserts.

The panel's pure parts moved to `lib/teamForum.js` so they can be tested without a
browser, following teamActivity.js and teamAdmin.js. Two of them are subtler than
they look:

  * `stripToText` decodes entities AFTER stripping tags, and `&amp;` last of all.
    Decoding first turns an author's literal "&lt;script&gt;" into a real tag the
    strip pass then deletes — silently losing text that was never dangerous.
  * `threadSummary` counts REPLIES, which is one fewer than `postCount`. Showing
    the raw count tells a reader a brand-new thread already has one reply.

**Three admin surfaces.** The forum settings screen gains the edit-window field
(0 = posts permanent once written). The reports queue is a new screen beside
Appeals — under moderation rather than under Teams, because a staffer working a
queue should have one place to work and `target_type` is deliberately open-ended,
so the next reportable thing arrives as a row rather than as another nav entry.
Its copy tells a member where a report lands and that reporting changes nothing,
because a member who expects a post to vanish and watches it stay reports it
again. There is no leader-facing view and there is not meant to be.

And the per-Team forum moderation ledger finally renders: the route and
`api.admin.teamForumModeration()` have both existed since phase 4 with nothing
calling them, which made `actor_role` — the column that keeps a leader's ordinary
housekeeping distinguishable from a staff intervention — readable only from a DB
client.

Co-Authored-By: Claude <noreply@anthropic.com>
fix(teams): let a post-moderation mistake reach the model that explains it
All checks were successful
PR Checks / bot-install (pull_request) Successful in 19s
PR Checks / client-build (pull_request) Successful in 30s
PR Checks / server-tests (pull_request) Successful in 34s
c970caee16
Found on the live rig. `moderatePost` answers `pin` with «"pin" applies to a
thread, not to a post» and an invented action with "Unknown moderation action" —
the distinction exists because they are different mistakes and a caller who made
the first one has a bug worth naming precisely.

The route's validator listed only the four actions a post accepts, so `pin` never
got there: it came back as a generic "Validation failed". The precise message was
written, documented, unit-tested — and unreachable through the API, which is the
worst of both, because the branch reads as live code and is only exercised by its
own test.

The validator now lists all eight and lets the model discriminate. Both answers
are 400, neither is a security boundary, and widening the list is not removing it
— an action outside the enum still stops at the validator, which the added route
test asserts alongside the `pin` case.

Nothing else the walk exercised needed changing. The whole phase 5 surface was
driven against a real server, real MariaDB and real sessions across four
identities — an ordinary member, a granted non-member guest, a Team leader and a
staffer — plus a browser pass over the forum panel, the reports queue, the
per-Team forum ledger and the settings screen. Notably confirmed live: a locked
thread refuses replies from all four identities at 409; a hidden post renders for
the leader and staff with Unhide and **no Edit control for anyone**; the report
queue answers 200 to staff and 403 to the leader, the member and the guest alike;
and turning the edit window down to 0 stops the author while leaving staff
unbounded.

Co-Authored-By: Claude <noreply@anthropic.com>
whitlocktech merged commit 0467c71ea1 into edge 2026-08-18 18:36:52 +00:00
whitlocktech deleted branch feature/teams-phase5-discussion 2026-08-18 18:36:53 +00:00
Sign in to join this conversation.
No description provided.