feat(teams): Team core — the reconciler, the four authority paths, and the impersonation controls #151

Merged
whitlocktech merged 6 commits from feat/teams-phase2-team-core into edge 2026-08-17 22:20:34 +00:00

6 Commits

Author SHA1 Message Date
cf2666e5bc feat(teams): the Team read API, the moderation routes, and Admin -> Teams
All checks were successful
PR Checks / bot-install (pull_request) Successful in 16s
PR Checks / client-build (pull_request) Successful in 24s
PR Checks / server-tests (pull_request) Successful in 8m56s
The eighteen routes of docs/website/TEAMS.md §2.11, their OpenAPI annotations,
and the staff screen that drives them.

Two rules shape the read model. Hidden means absent from every public surface --
the index, the lookup and the roster alike, and a hidden Team 404s
indistinguishably from one that does not exist, because "absent" includes not
confirming it is there. And staleness is surfaced rather than silent: every
public payload carries { configured, stale, lastSyncAt }, so a page can say how
recently the projection was confirmed instead of presenting stale data as
current.

The public roster withholds both the member key and the user id -- one is a
game-internal identifier, the other names a site account. `linked` answers the
only question a public page has without publishing which account. The module's
per-audience field projection is phase 3's; this is a conservative core one.

The §2.9 gate is enforced per REQUEST, not per route. A moderator may call all
eighteen; three of them mean something different when they do, and the server
decides from the role it re-validates on every request rather than from a token
claim. The client has no "file as request" argument to get wrong.

Found by booting the real server against the real database, and not by any test:
**the index and the by-slug lookup disagreed about what exists.** listPublic was
keyed on a registered team provider while findBySlug is not, so with no module
installed `/teams` returned an empty list while `/teams/:slug/members` served a
full roster -- the index denying a Team that direct URLs answered for in full.
The rows are core's and they outlive the module that filled them: an uninstalled
module leaves a projection that is unmaintained, not one that stopped existing,
and `configured: false` is how a client learns that. The read side no longer
takes the provider into account at all. There is now a test named for the
property.

Also verified live: the public routes answer anonymously, an unknown and a hidden
slug both 404, the player and admin tiers 401 an anonymous caller, a seeded
roster projects correctly, and the reconciler logs that it is staying idle with
no provider registered rather than failing a boot.

Process obligations, all done: #swagger.* annotations on every route, `npm run
swagger` regenerated (18 paths in the spec, no dangling $refs, and the schemas
they reference added), `npm run routes:manifest` regenerated -- additions only,
184 public routes -- and BACKEND_DESIGN.md updated across the schema section and
all three tier tables.

Admin -> Teams follows the ModulesAdmin precedent: everything that decides what a
row SAYS lives in lib/teamAdmin.js, which is plain JS with tests, and the view
renders it. That split earns itself here specifically -- the screen's job is to
make "the shard has no Teams" and "core has not been able to ask for two hours"
impossible to confuse, and those two produce the same empty table. The four
freshness states are named and tested for exactly that reason, and the last
provider error is shown verbatim rather than paraphrased.

The button labels follow the caller's role: a moderator sees "Request publish",
so the pending result is not a surprise. Hiding is offered to everyone with no
gate, matching the server.

Server 894 passed, client 206 passed, client build clean. 17 route tests, 20
client display tests.

Refs docs/website/TEAMS.md §2.11, Part 12 phase 2

Co-Authored-By: Claude <noreply@anthropic.com>
2026-08-17 15:27:02 -05:00
8fe2e01466 feat(teams): reserved-name screening, auto-hide, and the admin-approval gate
The one place untrusted game data becomes a public page (docs/website/TEAMS.md
§2.8), and the gate on releasing it (§2.9).

A Team's name is written by a player, in the game, with no review, and this
platform turns it into a public page, a URL and eventually a Discord channel
name. Someone naming their guild "Admin" or "<Brand> Staff" gets an
official-looking page on the operator's own site for free.

Hide, never reject. Core cannot refuse a name -- the guild already exists in the
game and core is a mirror of it, not an authority over it. A match hides the Team
from public surfaces and files it in a review queue, and it keeps working
completely for its own members: their forum, their grants, their notifications.
The people in it are not being punished for a name their leader chose.

That asymmetry -- a false positive costs a human glance, a false negative costs
an impersonated staff page -- is what lets the matcher be conservative. It is not
licence to be sloppy the other way: a check that fires on "Badminton" gets
switched off, and then the real cost is paid in full. So matching is whole WORDS
after normalisation, never substrings, following the precedent
scripts/checkModuleIdentifiers.js set for exactly this reason.

Three matcher gaps found by writing the tests, all real impersonation vectors:

  - "Guild of Moderators" did not match `moderator`. Only a trailing s off the
    WHOLE term is stripped, so "Nomads" still does not match `mod`.
  - "G.M." normalises to two single-letter words and matched nothing. A run of
    two or more single-letter words is now also offered joined. Deliberately not
    a whole-name condensation, which would re-admit substring matching.
  - The multi-word condensed form was already handled and is what makes
    "RunicGateway" match the two-word term -- the form an impersonator would
    reach for, since it is what the Gitea org and every URL use.

Terms resolve at CHECK time, never baked in, so renaming a deployment protects
the new name without a redeploy. A failed settings read falls back to the static
role and project terms rather than to an empty list: screening fewer terms is
bad, screening none is the whole hole.

Re-screening runs on every reconcile, over names no human has ruled on. Names are
immutable per row, so it only ever changes an outcome when the TERM LIST changed
-- an operator adding one, or a rename -- which is exactly what a create-time-only
check would miss forever. `name_reviewed_at` is what makes a staff decision
sticky; without it an override would be undone every fifteen minutes.

The gate is scoped to three actions because they publish untrusted game-sourced
strings, and to nothing else. Ordinary forum grants, leadership overrides,
archives and forum moderation still apply immediately and are audited. A
moderator initiating one files a pending request; an admin applies at once.
Never four-eyes on admins: users.role defaults to admin and `npm run seed`
creates exactly one, so most deployments have precisely one and a second-approver
rule would wedge them with no way out.

Hiding is deliberately NOT gated. Publishing untrusted data needs a second pair
of eyes; withdrawing it needs to be possible at once, by whoever is on duty.

Two concurrency details worth the review: a decision moves the row out of
`pending` under a guard and applies its effect only if the row actually moved,
so two admins clicking approve cannot double-apply or overwrite each other's
record; and a JSON payload is parsed defensively, because the driver returns
JSON columns already parsed on some versions and as a string on others.

Screening is stubbed in the reconciler's own tests -- it is a separate unit, and
the real call reads settings, which this suite must never do against a live
database. That was caught the hard way: the suite went from 11s to hanging, and
the cause was the reconciler reaching a dead pool through the new call.

44 tests in the reconciler file (up from 39), 19 for the matcher, 25 for the
gate. Full suite 877 passed, 0 failed.

Refs docs/website/TEAMS.md §2.8, §2.9, Part 12 phase 2

Co-Authored-By: Claude <noreply@anthropic.com>
2026-08-17 15:08:58 -05:00
bfd844e8fb feat(teams): the four-path access resolver and staff leadership overrides
The four authority paths of docs/website/TEAMS.md §2.5, and the rule that they
stay four: four tables answering four questions, and no resolver reads another
path's table.

  1. Is this account a member?        module  team_members
  2. Does this account lead the Team?  module  team_members.is_leader + override
  3. May it use the Team forum?        CORE    team_forum_grants OR path 1
  4. May it get external access?       CORE    derived, nothing of its own

The temptation this resists is collapsing 1 and 3 into one boolean. They answer
different questions about different populations: a forum grant may name any
Runic Gateway account, including one with no game identity at all -- that is the
point of it, since letting an unlinked guildmate into a forum must not require a
staff ticket. Reading "has forum access" as "is a member" would put that person
on the public roster, into every membership count, and into the external-platform
grant, which is where a modelling preference becomes an impersonation risk.

Path 4 is deliberately blind to path 3, and the reason is written down so nobody
"fixes" it: an integration cannot verify that an unlinked, forum-granted account
corresponds to a real game member, so it must not hand that account a privilege
on a platform where impersonation has consequences. A forum is a room on the
operator's own site with a known moderator; a Discord role is an identity claim
in someone else's space.

Leadership overrides are applied ON TOP of the synced value at read time, never
written into the projection. The sync owns that column and rewrites it every
interval, so an override stored there would be undone fifteen minutes after
staff set it -- which is the whole reason §2.5.1 is a separate table. The roster
carries both the resolved answer and `is_leader_synced`, so an admin sees that a
decision was made rather than being shown it as fact.

Three tests are named INVARIANT rather than for behaviour, because what they
protect is structural and a reasonable-looking refactor destroys it silently: a
grant never writes the membership projection, a granted user is absent from the
roster, and a grant does not confer external eligibility. None of those failures
appears on a screen as a bug -- the first shows up as a stranger on a public
roster, the second as a Discord role handed to an account nobody can tie to a
real player.

Every unit test here stubs the db layer, so the SQL itself was verified
separately: all 44 statements across teams.db.js and teamAccess.db.js were run
against MariaDB 11 with a throwaway module id and cleaned up after. That run
also confirmed live what the reconciler's tests could only assert against a
stub -- an upsert does not overwrite is_leader, a revoked grant frees the unique
key for a new one while the ledger keeps both, and an archived team stays
resolvable at its old slug while its external_id is free for the successor row.

19 tests. Full suite 828 passed, 0 failed.

Refs docs/website/TEAMS.md §2.5, §2.5.1, §2.6, Part 12 phase 2

Co-Authored-By: Claude <noreply@anthropic.com>
2026-08-17 14:57:23 -05:00
92631347f9 feat(teams): the reconciler, its four refusal gates, and ctx.teams (API 1.6.0)
Core's projection of the module's Teams, kept in step (docs/website/TEAMS.md
§2.4), plus the two ctx members a module pushes through.

The four gates are the file, and each is invariant 1 in a different costume --
module unavailability is staleness, never emptiness:

  1. getTeams() not ok           -> record the failure, touch NOTHING, return.
  2. ok but empty, core holds >=1 -> quarantine; apply only if the NEXT
                                    authoritative answer, an interval later,
                                    agrees.
  3. getTeamMembers() not ok      -> that Team's roster untouched and stale; the
                                    other Teams sync normally.
  4. ok but zero members, had some -> the same two-strikes quarantine, per Team.

Gates 2 and 4 exist because an authoritative-looking empty answer during a cold
start is the one failure indistinguishable from a real wipe. "Every Team on the
shard disbanded at once" costs one interval to confirm; getting it wrong empties
every roster on the site.

Events are an optimisation, never the source of truth. Member and leadership
deltas apply at once for a Team core already knows; team.created and
team.disbanded only ask for a run. §2.2 scopes archival to an authoritative full
list, so a repeated or spurious disband event costs a reconcile rather than a
Team -- and a Team invented from a delta would have no name, no roster and no
leaders anyway.

Two columns TEAMS.md did not contemplate, both on `teams`:

  - roster_synced_at, because team_sync_state holds one row per MODULE and gate 3
    leaves ONE Team behind while the others sync. Without a per-Team stamp that
    Team's page would report the module's last success as its own -- exactly the
    staleness the gate exists to surface.

  - members_empty_since, gate 4's per-Team quarantine. The twin of
    team_sync_state.pending_empty_since, which is per module and cannot express it.

One real bug found by its own test. The roster upsert was writing is_leader, so a
refused getTeamLeaders() left every member demoted -- the roster had already
written `leader: false` before the authoritative call was even made. §2.5 is
explicit that path 2 is answered by getTeamLeaders(), so is_leader is now set on
INSERT only (seeding a Team so it is not leaderless while that call fails) and
moved afterwards by setLeaders() alone. Two writers for one column was the whole
defect.

MODULE_API_VERSION 1.6.0 on both halves -- they state one contract and a module
declares one coreApi range. The number covers the whole Team surface per Part 11;
the members arrive by phase. registerTeamProvider, ctx.teams.publish and
ctx.teams.reconcile are live. ctx.teams.activity.push (§4, phase 3) and
api.registerSlashCommands (§7.1, phase 7) are present and THROW with a sentence
naming their phase, rather than being absent or silently accepting data into
tables that do not exist yet.

39 tests here, and the ctx surface guard in moduleLoader.test.js updated -- it
caught the addition, which is what it is for. Server 809 passed, client 192
passed, 0 failed.

Refs docs/website/TEAMS.md §2.2, §2.3, §2.4, Part 11, Part 12 phase 2

Co-Authored-By: Claude <noreply@anthropic.com>
2026-08-17 14:53:52 -05:00
8b63ffc725 feat(modules): registerTeamProvider, and a call path that cannot answer "empty"
The registration a module uses to become the authoritative source of Teams
(docs/website/TEAMS.md §2.3), plus the wrapper core calls it through.

registerTeamProvider is the first registration where core CALLS THE MODULE and
waits for an answer. Every existing one is either the module claiming a mount or
core notifying it; the closest precedent is registerAnnounceLeg's dispatch, and
this is modelled on it rather than invented. It also holds a single value rather
than a map, unlike every other registry: Teams have one authoritative source by
construction, and two modules answering "what teams exist" would produce two
disjoint sets under one `teams` table with no rule for merging them. A second
registration is therefore a collision, named against the module that holds it.

teamProvider.js is where invariant 1 -- module unavailability is staleness,
never emptiness -- is actually enforced. It is deliberately generous about what
counts as a failure: a rejected promise, a synchronous throw, a timeout, a
non-object, a bare array, a missing `ok`, or a structurally malformed row all
leave as the same `{ ok: false }` a module would have sent on purpose. There is
no shape a broken provider can produce that arrives at the reconciler looking
like an authoritative empty list -- which is the entire argument for the
envelope, since a bare array has exactly one such shape and it is the one a
module returns while its sidecar is still connecting.

A malformed row fails the whole call rather than being dropped. Salvaging is the
dangerous option: one unreadable member quietly omitted from a roster is
indistinguishable, downstream, from that member having left, and the sync would
mark them departed on the strength of a broken payload. Refusing costs one stale
interval.

The deadline timer is unreffed as well as cleared. Clearing covers the case
where the race settles; it cannot cover a module promise that never settles at
all, where nothing exists to clear until the deadline fires. Caught by the test
file taking 10.2s to run 265ms of assertions -- the same class of bug as the
mariadb pool that used to hold the suite open (test/_setup.js). 292ms now.

28 tests. Full suite 770 passed, 0 failed.

Refs docs/website/TEAMS.md §2.3, Part 12 phase 2

Co-Authored-By: Claude <noreply@anthropic.com>
2026-08-17 14:44:45 -05:00
225663d62e feat(teams): core schema for Teams, membership, sync state and moderation
The six core tables Team core is built on (docs/website/TEAMS.md §2.1, §2.5,
§2.5.1, §2.9), plus the §2.10 account-deletion decisions expressed as foreign
keys rather than left to whatever the defaults happened to be.

Every table is core-internal (§10.3): a module populates them through the team
provider and must never read or write one directly. They carry no <moduleId>_
prefix, correctly -- MODULE_API.md §2.6's prefix rule binds modules, and these
are core's.

team_forum_grants lands in this phase rather than in phase 4, so the four-path
resolver is written once and its non-contamination tests are real. Nothing
writes it yet; the grant/revoke flow, the per-Team cap and the leader UI are
phase 4's.

Two departures from the SQL as TEAMS.md sketched it, both recorded in the file:

  - team_forum_grants.user_id is nullable with ON DELETE SET NULL, following
    §2.10 (the audit trail of who granted whom must survive the account) rather
    than §2.5's CASCADE.

  - its uniqueness marker is derived from revoked_at alone, with user_id moved
    into the unique KEY. §2.5's `active_user AS (IF(revoked_at IS NULL, user_id,
    NULL))` cannot coexist with the line above: MariaDB refuses ON DELETE SET
    NULL on a foreign key whose column is a base column of a STORED generated
    column (error 1901). The semantics are identical -- at most one active grant
    per (team, user), unlimited revoked rows.

Verified by running ensureSchema() against MariaDB 11: all six tables create,
both generated columns materialise, and every foreign key's delete rule matches
§2.10's table. The uniqueness encoding was checked directly -- a second active
grant for the same (team, user) is rejected 1062 while revoked rows accumulate
freely.

Refs docs/website/TEAMS.md Part 12 phase 2

Co-Authored-By: Claude <noreply@anthropic.com>
2026-08-17 14:40:26 -05:00