From aa332eda8262b7fc10ff42ff98f27747fc720209 Mon Sep 17 00:00:00 2001 From: wtclaude Date: Mon, 17 Aug 2026 20:15:18 -0500 Subject: [PATCH 1/5] feat(teams): the activity feed, its two writers and its retention MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TEAMS.md Part 4. `team_activity` takes items from two sources and treats them identically on the read path: core writes its own membership and rename items with source='core', and a module pushes game items through `ctx.teams.activity.push`, which stops throwing and starts working. Core writing here too is deliberate — the rendering path is exercised by core's own content from day one, so the feed is never empty on a deployment whose module pushes nothing. Three rules shape the model: - core never composes a summary. It arrives already rendered and is stored verbatim; core cannot phrase "gained 15,000 gold" for a game whose vocabulary it does not know. - visibility fails closed. An item with no stated visibility is `members`. - a push never throws at its call site. It is called from inside a game-event handler, and a storage problem of core's must not become the module's control flow. Core emits four of the five kinds §4.2 names — `core.forum.thread` has nothing to emit it until the forum lands in phase 4 — and emits none of them for a Team's FIRST roster: importing a 155-member guild is one Team arriving, not 155 people joining, and a join per member would bury every real event under the import and reach the row cap on day one. Retention ships with the feed rather than after someone notices. A nightly worker applies an age horizon and a per-Team row cap, both settings; either alone has a hole, since age lets one busy guild write a million rows inside the window and a cap keeps a dead Team's feed forever. The sync now reads member ROWS rather than keys, replacing the `memberKeys` call rather than adding to it: the feed needs each changing member's display name and prior `is_leader`, and the upsert is about to overwrite both. Co-Authored-By: Claude --- server/db/schema.sql | 40 +++ server/src/model/teams/teamActivity.db.js | 133 ++++++++ server/src/model/teams/teamActivity.model.js | 312 +++++++++++++++++++ server/src/model/teams/teamSync.model.js | 88 +++++- server/src/modules/loader.js | 21 +- server/src/server.js | 7 + server/src/utils/teamActivityPrune.js | 64 ++++ server/test/teamActivity.test.js | 271 ++++++++++++++++ server/test/teamSync.test.js | 173 ++++++++++ 9 files changed, 1100 insertions(+), 9 deletions(-) create mode 100644 server/src/model/teams/teamActivity.db.js create mode 100644 server/src/model/teams/teamActivity.model.js create mode 100644 server/src/utils/teamActivityPrune.js create mode 100644 server/test/teamActivity.test.js diff --git a/server/db/schema.sql b/server/db/schema.sql index 2808561..2fd7ec4 100644 --- a/server/db/schema.sql +++ b/server/db/schema.sql @@ -1051,6 +1051,46 @@ CREATE TABLE IF NOT EXISTS team_moderation_requests ( INDEX idx_tmr_queue (status, requested_at) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; +-- The per-Team activity feed (TEAMS.md §4.2, phase 3). Two writers, one table: +-- core writes its own membership and rename items with source='core', and a module +-- pushes game items through ctx.teams.activity.push with source=. That +-- core writes here too is deliberate — the rendering path is exercised by core's +-- own content from day one, so the feed is never empty on a deployment whose +-- module pushes nothing. +-- +-- `summary` is ALREADY-RENDERED text and core never composes one (§4.1). Core +-- cannot phrase "gained 15,000 gold" for a game whose vocabulary it does not know, +-- and a core that templated it would have re-acquired exactly the game semantics +-- the module system exists to remove. `kind` and `payload` are likewise opaque: +-- core stores and filters them, and only the module's `team.overview` slot renders +-- anything richer than the text. +CREATE TABLE IF NOT EXISTS team_activity ( + id BIGINT AUTO_INCREMENT PRIMARY KEY, + team_id INT NOT NULL, + source VARCHAR(32) NOT NULL, -- 'core' or a module id + kind VARCHAR(64) NOT NULL, -- namespaced ., opaque to core + summary VARCHAR(255) NOT NULL, -- module-rendered; core never composes one + -- Defaults to 'members' — fail closed. The module CHOOSES visibility per item; + -- core ENFORCES it on the read path. Same shape as a module owning the + -- public-safety filter for its push streams (MODULE_API.md §2.4). + visibility ENUM('public','members') NOT NULL DEFAULT 'members', + actor_member_key VARCHAR(191) NULL, + actor_user_id INT NULL, + payload JSON NULL, -- opaque; rendered only by the module's slot + occurred_at DATETIME NOT NULL, -- when it happened in the game, not when it arrived + -- Optional idempotence key. INSERT IGNORE against this unique index is the same + -- trick shard_events already uses, and it is what makes a sidecar reconnect + -- backfill safe: replaying a window of events re-posts nothing. + dedupe_key CHAR(40) NULL, + created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, + CONSTRAINT fk_team_activity_team FOREIGN KEY (team_id) REFERENCES teams(id) ON DELETE CASCADE, + -- Actor is SET NULL, not CASCADE (§2.10): deleting an account must not delete the + -- Team's history of what happened, only the attribution. + CONSTRAINT fk_team_activity_actor FOREIGN KEY (actor_user_id) REFERENCES users(id) ON DELETE SET NULL, + UNIQUE KEY uq_team_activity_dedupe (team_id, dedupe_key), + INDEX idx_team_activity_feed (team_id, occurred_at) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; + -- Migrations for databases created before the wiki upgrade. Each statement uses -- IF NOT EXISTS so re-running on every boot is a harmless no-op. New installs get -- these columns from the CREATE TABLE above; existing installs get them here. diff --git a/server/src/model/teams/teamActivity.db.js b/server/src/model/teams/teamActivity.db.js new file mode 100644 index 0000000..efd6f48 --- /dev/null +++ b/server/src/model/teams/teamActivity.db.js @@ -0,0 +1,133 @@ +// SQL for the per-Team activity feed (TEAMS.md §4.2). Statements only; every +// decision about what a caller may SEE lives in teamActivity.model.js. + +const { query } = require('../../utils/db') + +const ACTIVITY_COLUMNS = ` + id, team_id, source, kind, summary, visibility, + actor_member_key, actor_user_id, payload, occurred_at, created_at` + +/** + * Insert one item, idempotently when it carries a dedupe key. + * + * INSERT IGNORE against uq_team_activity_dedupe is what makes replay safe: a + * sidecar reconnect backfills a window of events it already delivered, and + * without this every reconnect would double-post the feed. The same trick + * `shard_events` uses, for the same reason. + * + * The unique key is (team_id, dedupe_key) and MariaDB treats NULL as distinct in + * a unique index, so items WITHOUT a key never collide with each other — an + * un-keyed push is always an insert, which is the documented contract (§4.1: + * `dedupeKey` is optional and "makes replay idempotent", so omitting it opts out). + * + * IGNORE would also swallow a genuine error — a bad FK, an over-long summary. The + * model validates and truncates before calling, so what reaches here can only fail + * on the dedupe key, and `affectedRows` reports which happened. + */ +async function insert(item) { + const res = await query( + `INSERT IGNORE INTO team_activity + (team_id, source, kind, summary, visibility, actor_member_key, actor_user_id, payload, occurred_at, dedupe_key) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + [ + item.teamId, + item.source, + item.kind, + item.summary, + item.visibility, + item.actorMemberKey, + item.actorUserId, + item.payload === null ? null : JSON.stringify(item.payload), + new Date(item.occurredAt), + item.dedupeKey, + ], + ) + return Number(res.affectedRows) > 0 +} + +/** + * One page of a Team's feed, already narrowed to the visibilities the caller may + * see. + * + * `visibilities` is always supplied by the model and never by a request + * parameter — a caller naming its own visibility filter is the whole bug this + * table's ENUM exists to prevent. Ordered newest first by `occurred_at`, the + * game's clock, not `created_at`: a backfill that arrives late still sorts where + * it happened. + */ +async function page(teamId, visibilities, { limit, offset }) { + const slots = visibilities.map(() => '?').join(', ') + return query( + `SELECT ${ACTIVITY_COLUMNS} FROM team_activity + WHERE team_id = ? AND visibility IN (${slots}) + ORDER BY occurred_at DESC, id DESC + LIMIT ? OFFSET ?`, + [teamId, ...visibilities, limit, offset], + ) +} + +/** Total matching rows, for the same filter — so a client can page honestly. */ +async function count(teamId, visibilities) { + const slots = visibilities.map(() => '?').join(', ') + const rows = await query( + `SELECT COUNT(*) AS n FROM team_activity WHERE team_id = ? AND visibility IN (${slots})`, + [teamId, ...visibilities], + ) + return Number(rows[0] ? rows[0].n : 0) +} + +/** Everything older than the retention horizon, across every Team. */ +async function deleteOlderThan(days) { + const res = await query( + 'DELETE FROM team_activity WHERE occurred_at < (NOW() - INTERVAL ? DAY)', + [days], + ) + return Number(res.affectedRows) || 0 +} + +/** + * Which Teams currently exceed the per-Team row cap, and by how much. + * + * Asked first so the trim only runs for Teams that need it. A feed fed by a game + * loop is the obvious unbounded-growth failure (§4.2), and on a shard with one + * busy guild and fifty quiet ones this keeps the nightly job proportional to the + * problem rather than to the number of Teams. + */ +async function overCap(cap) { + return query( + `SELECT team_id, COUNT(*) AS n FROM team_activity + GROUP BY team_id HAVING n > ?`, + [cap], + ) +} + +/** + * Trim one Team back to the newest `cap` rows. + * + * Expressed as "delete everything at or below the id of the cap-th newest row" + * rather than as a correlated subquery on the same table, which MariaDB refuses + * inside a DELETE (error 1093). The derived table is what makes it legal — the + * subquery is materialised before the delete runs. + */ +async function trimToCap(teamId, cap) { + const rows = await query( + `SELECT id FROM team_activity + WHERE team_id = ? ORDER BY occurred_at DESC, id DESC LIMIT 1 OFFSET ?`, + [teamId, cap], + ) + if (!rows[0]) return 0 + const res = await query( + 'DELETE FROM team_activity WHERE team_id = ? AND id <= ?', + [teamId, rows[0].id], + ) + return Number(res.affectedRows) || 0 +} + +module.exports = { + insert, + page, + count, + deleteOlderThan, + overCap, + trimToCap, +} diff --git a/server/src/model/teams/teamActivity.model.js b/server/src/model/teams/teamActivity.model.js new file mode 100644 index 0000000..f6d0e84 --- /dev/null +++ b/server/src/model/teams/teamActivity.model.js @@ -0,0 +1,312 @@ +// ── The per-Team activity feed (TEAMS.md Part 4) ─────────────────────────── +// +// Two writers, one table. A module pushes game items through +// `ctx.teams.activity.push` (§4.1); core writes its own membership and rename +// items directly (§4.2). Both land in `team_activity` with a `source`, and the +// read path treats them identically — which is the point of core writing here at +// all, since it means the rendering path is exercised from day one on a +// deployment whose module pushes nothing. +// +// **Three rules shape this file.** +// +// 1. *Core never composes a summary.* `summary` arrives already rendered and is +// stored verbatim (§4.1). Core cannot phrase "gained 15,000 gold" for a game +// whose vocabulary it does not know, and a core that templated it would have +// re-acquired the game semantics the module system exists to remove. Core's OWN +// five kinds are the sole exception, and they are about membership and renames +// — platform facts, not game ones. +// +// 2. *Visibility fails closed.* An item with no stated visibility is `members`, +// and the read path resolves what a caller may see from their access rather +// than from anything they send. +// +// 3. *A push never throws at its call site.* `ctx.teams.activity.push` is awaited +// by a module inside a game-event handler. A bad item is dropped and logged; +// an unknown Team is dropped and logged. The alternative — rejecting the batch +// — makes core's storage problem into the module's control flow, and the +// contract (MODULE_API.md §2.3) is that ctx pushes are fire-and-forget. + +const activityDb = require('./teamActivity.db') +const teamsDb = require('./teams.db') +const access = require('./teamAccess.model') +const settings = require('../settings/settings.model') + +const log = require('../../utils/logger')('teams') + +// Column widths from schema.sql. Truncating rather than refusing: an over-long +// summary is a module being verbose, not a module being wrong, and dropping the +// item would lose a real event over a display detail. +const MAX_SUMMARY = 255 +const MAX_KIND = 64 +const MAX_MEMBER_KEY = 191 +// CHAR(40) — a sha1 hex is the natural fit and what §4.1's example looks like, +// but the column is opaque and any stable string within the width works. +const MAX_DEDUPE = 40 + +const VISIBILITIES = ['public', 'members'] + +// Retention (§4.2). Both are settings so an operator can tighten a busy shard +// without a deploy; the defaults are the doc's. +const DEFAULT_RETAIN_DAYS = 90 +const DEFAULT_ROW_CAP = 2000 + +/** + * Core's own kinds (§4.2). + * + * `core.forum.thread` is named in the doc and lands with the forum in phase 4 — + * there is nothing to emit it from yet. The four here are all core knows how to + * say without asking a game anything. + */ +const CORE_KINDS = { + MEMBER_JOINED: 'core.member.joined', + MEMBER_LEFT: 'core.member.left', + LEADER_CHANGED: 'core.leader.changed', + TEAM_RENAMED: 'core.team.renamed', +} + +const clamp = (v, max) => (typeof v === 'string' && v.trim() ? v.trim().slice(0, max) : null) + +/** + * Normalise one pushed item, or return null to drop it. + * + * `teamId` is resolved by the caller, not carried on the item: a module names its + * own `externalId` and core maps it (§4.1), so a module can never write into + * another module's Team by guessing an integer. + */ +function normalise(item, source, teamId) { + if (!item || typeof item !== 'object') return null + + const kind = clamp(item.kind, MAX_KIND) + const summary = clamp(item.summary, MAX_SUMMARY) + // Both are load-bearing and neither has a safe default: an item with no kind + // cannot be filtered or rendered by a slot, and one with no summary is a blank + // row on a public page. + if (!kind || !summary) return null + + // `occurredAt` is the game's clock and the feed's sort key. A missing or + // unparseable one becomes now — the item is real even when its timestamp is + // not, and dropping it would lose an event over metadata. + const occurredAt = Number.isFinite(item.occurredAt) ? Number(item.occurredAt) : Date.now() + + return { + teamId, + source, + kind, + summary, + visibility: VISIBILITIES.includes(item.visibility) ? item.visibility : 'members', + actorMemberKey: clamp(item.actorMemberKey, MAX_MEMBER_KEY), + // Resolved BY THE MODULE, like every other user id crossing this boundary + // (§2.3) — core takes the number and never looks it up. + actorUserId: Number.isInteger(item.actorUserId) && item.actorUserId > 0 ? item.actorUserId : null, + payload: item.payload && typeof item.payload === 'object' ? item.payload : null, + occurredAt, + dedupeKey: clamp(item.dedupeKey, MAX_DEDUPE), + } +} + +/** + * `ctx.teams.activity.push` — a module's whole write access to the feed. + * + * Items name their Team by the module's own `externalId`, and only ACTIVE Teams + * owned by THAT module resolve. An archived Team is deliberately not writable: its + * feed is a read-only record of what happened before the rename or the disband + * (§2.2), and letting a late-arriving event append to it would make a closed + * record grow. + * + * Returns the number of items actually stored. Dropped items are logged with the + * reason and never raised — see rule 3 above. + */ +async function push(source, items) { + if (!Array.isArray(items)) { + log.warn('teams activity push: not an array', { source }) + return 0 + } + if (!items.length) return 0 + + // One lookup per distinct externalId, not one per item: a champion spawn + // completing pushes a batch for a single Team, and re-resolving it per item + // would be a query per row. + const teamIds = new Map() + let stored = 0 + let dropped = 0 + + for (const item of items) { + const externalId = item && typeof item.externalId === 'string' ? item.externalId.trim() : '' + if (!externalId) { dropped += 1; continue } + + if (!teamIds.has(externalId)) { + // eslint-disable-next-line no-await-in-loop + const row = await teamsDb.findActive(source, externalId) + teamIds.set(externalId, row ? row.id : null) + } + const teamId = teamIds.get(externalId) + if (!teamId) { dropped += 1; continue } + + const normalised = normalise(item, source, teamId) + if (!normalised) { dropped += 1; continue } + + // eslint-disable-next-line no-await-in-loop + const inserted = await activityDb.insert(normalised) + // A dedupe collision is a SUCCESSFUL no-op, not a drop — it is the mechanism + // working. Counted as stored so a module replaying a backfill does not read + // its own idempotence as data loss. + if (inserted) stored += 1 + } + + if (dropped) { + log.warn('teams activity push: dropped items', { source, dropped, offered: items.length }) + } + return stored +} + +/** + * Core's own write path (§4.2), used by the reconciler and the rename rule. + * + * Separate from `push` because core names a Team by its own primary key — it is + * already holding the row — and because core's items are always `public`: a + * member joining or a Team being renamed is exactly what a public Team page is + * for. Nothing here is game vocabulary. + */ +async function logCore({ teamId, kind, summary, actorMemberKey = null, actorUserId = null, occurredAt = Date.now(), dedupeKey = null }) { + if (!teamId || !kind || !summary) return false + return activityDb.insert({ + teamId, + source: 'core', + kind: clamp(kind, MAX_KIND), + summary: clamp(summary, MAX_SUMMARY), + visibility: 'public', + actorMemberKey: clamp(actorMemberKey, MAX_MEMBER_KEY), + actorUserId: Number.isInteger(actorUserId) && actorUserId > 0 ? actorUserId : null, + payload: null, + occurredAt, + dedupeKey: clamp(dedupeKey, MAX_DEDUPE), + }) +} + +/** + * Which visibilities a caller may see (§4.3). + * + * `members` items go to members and to forum-granted users — the same two + * authority paths `forumAccess` already resolves, reused rather than re-derived + * so the feed can never disagree with the forum about who is inside a Team. + * Anyone else, including every anonymous caller, sees `public` only. + */ +async function visibilitiesFor(teamId, userId) { + if (!userId) return ['public'] + const resolved = await access.forumAccess(teamId, userId) + return resolved.allowed ? ['public', 'members'] : ['public'] +} + +/** The rendered shape. `payload` rides along for the module's slot (§4.3). */ +function publicItem(row) { + return { + id: Number(row.id), + source: row.source, + kind: row.kind, + summary: row.summary, + visibility: row.visibility, + occurredAt: row.occurred_at, + payload: row.payload ?? null, + } +} + +/** + * One page of a Team's feed for one viewer. + * + * A HIDDEN Team's feed is not served publicly, for the same reason its roster is + * not (§2.8.3): hidden means absent from every public surface, and a feed that + * answered while the page 404s would republish the suppressed name in every + * `core.team.renamed` summary. + */ +async function feedFor(slug, userId, { limit = 50, offset = 0 } = {}) { + const row = await teamsDb.findBySlug(slug) + if (!row) return null + + const visibilities = await visibilitiesFor(row.id, userId) + // A member of a hidden Team still sees its feed — suppression is a + // public-surface rule, and a member is not a member of the public (§2.11). + if (row.hidden && visibilities.length === 1) return null + + const [rows, total] = await Promise.all([ + activityDb.page(row.id, visibilities, { limit, offset }), + activityDb.count(row.id, visibilities), + ]) + return { + items: rows.map(publicItem), + total, + limit, + offset, + // So a client can render "members-only items are hidden" rather than + // presenting a filtered feed as the whole one. + scope: visibilities.includes('members') ? 'members' : 'public', + } +} + +// ── Retention (§4.2) ─────────────────────────────────────────────────────── + +const RETAIN_KEY = 'team_activity_retain_days' +const CAP_KEY = 'team_activity_row_cap' + +/** + * Read both limits, falling back to the defaults on anything unreadable. + * + * Wrapped in a try like `teamSync.intervalSeconds`, and for the same reason: this + * runs on a timer with nobody watching, and a settings table that is briefly + * unavailable must yield the default rather than an exception that kills the + * nightly job. A misconfigured value fails the same way — a zero or a negative + * retention would delete the whole feed, so it is rejected rather than honoured. + */ +async function retentionConfig() { + let rawDays + let rawCap + try { + ;[rawDays, rawCap] = await Promise.all([settings.get(RETAIN_KEY), settings.get(CAP_KEY)]) + } catch { + return { days: DEFAULT_RETAIN_DAYS, cap: DEFAULT_ROW_CAP } + } + const days = Number.parseInt(rawDays, 10) + const cap = Number.parseInt(rawCap, 10) + return { + days: Number.isFinite(days) && days > 0 ? days : DEFAULT_RETAIN_DAYS, + cap: Number.isFinite(cap) && cap > 0 ? cap : DEFAULT_ROW_CAP, + } +} + +/** + * The nightly prune: an age horizon AND a per-Team row cap. + * + * Both, because either alone has a hole. Age alone lets one busy guild write a + * million rows inside the window; a cap alone keeps a dead Team's feed forever. + * Unbounded growth on a per-Team feed fed by a game loop is the obvious failure + * here and it is cheaper to bound it now than to discover it at cutover. + */ +async function prune() { + const { days, cap } = await retentionConfig() + const byAge = await activityDb.deleteOlderThan(days) + + let byCap = 0 + const over = await activityDb.overCap(cap) + for (const row of over) { + // eslint-disable-next-line no-await-in-loop + byCap += await activityDb.trimToCap(row.team_id, cap) + } + + if (byAge || byCap) log.info('teams activity prune', { byAge, byCap, days, cap }) + return { byAge, byCap, days, cap } +} + +module.exports = { + push, + logCore, + feedFor, + visibilitiesFor, + publicItem, + prune, + retentionConfig, + RETAIN_KEY, + CAP_KEY, + CORE_KINDS, + VISIBILITIES, + DEFAULT_RETAIN_DAYS, + DEFAULT_ROW_CAP, +} diff --git a/server/src/model/teams/teamSync.model.js b/server/src/model/teams/teamSync.model.js index 1847470..067f41f 100644 --- a/server/src/model/teams/teamSync.model.js +++ b/server/src/model/teams/teamSync.model.js @@ -32,6 +32,7 @@ const teamsDb = require('./teams.db') const teamProvider = require('./teamProvider') const moderation = require('./teamModeration.model') +const activity = require('./teamActivity.model') const { slugify, uniqueSlug } = require('./teamSlug') const settings = require('../settings/settings.model') const log = require('../../utils/logger')('teams') @@ -130,9 +131,66 @@ async function applyRename(moduleId, existing, team) { log.info('team renamed; previous row archived', { externalId: team.externalId, from: existing.name, to: team.name, archivedId: existing.id, successorId, }) + // §4.2's `core.team.renamed`, written to the SUCCESSOR rather than to the row + // that was renamed: the archived row is a read-only record of what happened + // before the rename (§2.2), and the person who wants to know a Team used to be + // called something else is looking at the live page. + // + // The old name is core's own, not game-sourced text a module handed us this + // run — it is the `name` column core has been serving all along — so §2.9's + // approval gate does not apply. It can still be a name staff suppressed, which + // is why a hidden Team's feed is not served publicly (teamActivity.feedFor). + await activity.logCore({ + teamId: successorId, + kind: activity.CORE_KINDS.TEAM_RENAMED, + summary: `Renamed from ${existing.display_name_override || existing.name}`, + dedupeKey: `renamed:${existing.id}`, + }).catch((err) => log.warn('rename activity not recorded', { message: err.message })) return successorId } +/** Never a game-internal member key on a public page: that identifier is not published (§3.2). */ +const memberLabel = (row) => (row && row.display_name) || 'A member' + +/** + * Core's own membership items for one roster run (§4.2). + * + * **Suppressed on a Team's FIRST roster.** Importing a 155-member guild is one + * Team arriving, not 155 people joining, and emitting a join per member would + * bury every real event under the import and blow through the row cap on day one. + * `roster_synced_at IS NULL` is exactly "core has never held a roster for this + * Team", so the same condition covers a newly created Team and a newly installed + * module adopting an existing one. + * + * Never throws: the feed is a rendering of the sync, and a feed write failing + * must not abort the sync that is the actual source of truth. + */ +async function logRosterActivity(team, { joined, left, promoted, demoted }) { + if (!team.roster_synced_at) return + + const items = [ + ...joined.map((row) => ({ kind: activity.CORE_KINDS.MEMBER_JOINED, row, verb: 'joined' })), + ...left.map((row) => ({ kind: activity.CORE_KINDS.MEMBER_LEFT, row, verb: 'left' })), + ...promoted.map((row) => ({ kind: activity.CORE_KINDS.LEADER_CHANGED, row, verb: 'became a leader' })), + ...demoted.map((row) => ({ kind: activity.CORE_KINDS.LEADER_CHANGED, row, verb: 'stepped down as a leader' })), + ] + + for (const { kind, row, verb } of items) { + try { + // eslint-disable-next-line no-await-in-loop + await activity.logCore({ + teamId: team.id, + kind, + summary: `${memberLabel(row)} ${verb}`, + actorMemberKey: row.member_key, + actorUserId: row.user_id ?? null, + }) + } catch (err) { + log.warn('roster activity not recorded', { teamId: team.id, kind, message: err.message }) + } + } +} + /** * Sync one Team's roster and leadership. Gates 3 and 4 live here. * @@ -152,7 +210,13 @@ async function syncRoster(team) { return false } - const known = await teamsDb.memberKeys(team.id) + // The full rows rather than just the keys: the activity feed needs the display + // name and the prior `is_leader` of everyone who is about to change, and both + // are gone once the upsert below has run. One read either way — this replaces + // the `memberKeys` call rather than adding to it. + const knownRows = await teamsDb.membersByTeam(team.id) + const knownByKey = new Map(knownRows.map((row) => [row.member_key, row])) + const known = knownRows.map((row) => row.member_key) // Gate 4, the per-Team twin of gate 2. if (answer.complete && answer.members.length === 0 && known.length > 0) { @@ -184,17 +248,35 @@ async function syncRoster(team) { }) } + // Anyone the module reports that core was not already holding. Read from the + // module's shape, since a joiner has no row yet. + const joined = answer.members + .filter((m) => !knownByKey.has(m.memberKey)) + .map((m) => ({ member_key: m.memberKey, display_name: m.displayName, user_id: m.userId })) + // Removals only from a COMPLETE answer. `complete: false` means "valid but // partial", so additions and updates apply and nothing is taken away. + let left = [] if (answer.complete) { const seen = new Set(answer.members.map((m) => m.memberKey)) - await teamsDb.markDeparted(team.id, known.filter((key) => !seen.has(key))) + const departedKeys = known.filter((key) => !seen.has(key)) + left = departedKeys.map((key) => knownByKey.get(key)) + await teamsDb.markDeparted(team.id, departedKeys) } // Leadership is a separate question with a separate answer, and a provider that // cannot answer it leaves the synced value alone rather than demoting everyone. const leaders = await teamProvider.getTeamLeaders(team.external_id) + let promoted = [] + let demoted = [] if (leaders.ok) { + // Diffed against the PRIOR rows, before setLeaders overwrites them. A member + // who joined this run as a leader is reported as joining, not as being + // promoted — they were never anything else here. + const nowLeader = new Set(leaders.leaders) + const departed = new Set(left.map((row) => row && row.member_key)) + promoted = knownRows.filter((row) => nowLeader.has(row.member_key) && !row.is_leader) + demoted = knownRows.filter((row) => !nowLeader.has(row.member_key) && row.is_leader && !departed.has(row.member_key)) await teamsDb.setLeaders(team.id, leaders.leaders) } else { log.warn('leadership left untouched; provider could not answer', { @@ -203,6 +285,8 @@ async function syncRoster(team) { } await teamsDb.recount(team.id) + // Read before `markRosterSynced` moves the stamp this decision turns on. + await logRosterActivity(team, { joined, left: left.filter(Boolean), promoted, demoted }) await teamsDb.markRosterSynced(team.id) return true } diff --git a/server/src/modules/loader.js b/server/src/modules/loader.js index c1d4370..9d42d1e 100644 --- a/server/src/modules/loader.js +++ b/server/src/modules/loader.js @@ -120,6 +120,7 @@ function buildCtx(id, moduleRoot) { const activity = require('../model/activity/activity.model') const users = require('../model/users/users.model') const teams = require('../model/teams/teamSync.model') + const teamActivity = require('../model/teams/teamActivity.model') const { makeLimiter, accountChangeLimiter } = require('../middleware/rateLimit') /* eslint-enable global-require */ @@ -190,14 +191,20 @@ function buildCtx(id, moduleRoot) { teams: { publish: (event) => teams.publish(event), reconcile: (opts) => teams.request(opts), - // §4's activity feed, which lands with the Team pages in phase 3. Declared - // in 1.6.0 alongside the rest of the Team surface; calling it before phase 3 - // throws rather than silently accepting items into a table that does not - // exist yet. + // §4's activity feed (phase 3). `source` is bound to the CALLING module and + // is never taken from the item — a module writes its own items, under its + // own name, and items name their Team by the module's own `externalId`, so + // there is no id a module could send that reaches another module's Team. + // + // Like `publish` and `reconcile` above, a failure here never reaches the + // module: this is called from inside a game-event handler, and a storage + // problem of core's must not become the module's control flow. A rejected + // write is logged and the promise still resolves. activity: { - push: () => { - throw new Error('ctx.teams.activity.push is not available until the Team activity feed lands (TEAMS.md §4)') - }, + push: (items) => teamActivity.push(id, items).then( + (stored) => { void stored }, + (err) => { log.error('ctx.teams.activity.push failed', { module: id, message: err.message }) }, + ), }, }, // One function, for one caller: the `admin.users.detail` slot router needs diff --git a/server/src/server.js b/server/src/server.js index 9de900a..4474214 100644 --- a/server/src/server.js +++ b/server/src/server.js @@ -9,6 +9,7 @@ const http = require('http') // now because none of it reaches the loader's scan. const botScore = require('./middleware/botScore') const announceWorker = require('./utils/announceWorker') +const teamActivityPrune = require('./utils/teamActivityPrune') const { ensureSchema, close } = require('./utils/db') const { seedDefaults, createInitialAdminFromEnv } = require('../db/seed') const settings = require('./model/settings/settings.model') @@ -150,6 +151,11 @@ async function start() { // retry per leg. No-op until a news post is actually published. announceWorker.start() + // Bound the per-Team activity feed (TEAMS.md §4.2). A feed fed by a game loop + // is the obvious unbounded-growth failure, so retention starts with the feed + // rather than after someone notices. No-op on a deployment with no Teams. + teamActivityPrune.start() + setupShutdown(server, internalServer) } @@ -167,6 +173,7 @@ function setupShutdown(server, internalServer) { await moduleLifecycle.shutdown() botScore.stopSweeper() // stop the bot-store cleanup interval announceWorker.stop() // stop the news-announcement dispatcher poller + teamActivityPrune.stop() // stop the Team activity retention timer server.close(() => log.info('http server closed')) if (internalServer) internalServer.close(() => log.info('internal http server closed')) try { diff --git a/server/src/utils/teamActivityPrune.js b/server/src/utils/teamActivityPrune.js new file mode 100644 index 0000000..6f2e71e --- /dev/null +++ b/server/src/utils/teamActivityPrune.js @@ -0,0 +1,64 @@ +// ── Team activity retention worker ────────────────────────────────────────── +// +// TEAMS.md §4.2's nightly prune: an age horizon (`team_activity_retain_days`, +// default 90) and a per-Team row cap (`team_activity_row_cap`, default 2000). +// Both live in `settings`, so an operator can tighten a busy shard without a +// deploy. +// +// Same in-process shape as utils/announceWorker and middleware/botScore's +// sweeper — setInterval + unref + stop(), wired into server.js start/shutdown. +// There is no cron in this stack and adding one for a single daily DELETE would +// be a dependency to justify at every future upgrade. +// +// **The first run is delayed rather than immediate.** A prune at boot would put a +// table-wide DELETE in front of the first request on every restart, and a +// deployment that is crash-looping would run it on every loop. Five minutes in is +// past the point where a boot has either succeeded or failed. + +const teamActivity = require('../model/teams/teamActivity.model') +const log = require('./logger')('teams') + +// Nightly, per §4.2. Not aligned to a wall-clock hour: the work is proportional +// to what arrived rather than to when it is done, and pinning it to 03:00 would +// mean a process that restarts each afternoon never prunes at all. +const INTERVAL_MS = Number(process.env.TEAM_ACTIVITY_PRUNE_MS) || 24 * 60 * 60 * 1000 +const FIRST_RUN_MS = Number(process.env.TEAM_ACTIVITY_PRUNE_DELAY_MS) || 5 * 60 * 1000 + +let timer = null +let firstRun = null + +/** One prune. Never throws — it runs on a timer with nobody to catch it. */ +async function tick() { + try { + return await teamActivity.prune() + } catch (err) { + log.error('team activity prune failed', { message: err.message }) + return null + } +} + +function start() { + if (timer || firstRun) return timer + firstRun = setTimeout(() => { + firstRun = null + tick() + timer = setInterval(() => { tick() }, INTERVAL_MS) + if (timer.unref) timer.unref() + }, FIRST_RUN_MS) + if (firstRun.unref) firstRun.unref() + log.info('team activity retention started', { intervalMs: INTERVAL_MS, firstRunMs: FIRST_RUN_MS }) + return timer +} + +function stop() { + if (firstRun) { + clearTimeout(firstRun) + firstRun = null + } + if (timer) { + clearInterval(timer) + timer = null + } +} + +module.exports = { start, stop, tick, INTERVAL_MS, FIRST_RUN_MS } diff --git a/server/test/teamActivity.test.js b/server/test/teamActivity.test.js new file mode 100644 index 0000000..58fb32b --- /dev/null +++ b/server/test/teamActivity.test.js @@ -0,0 +1,271 @@ +// The per-Team activity feed (docs/website/TEAMS.md Part 4). +// +// The db layer is stubbed and an in-memory table stands in for `team_activity`, +// so these are assertions about the RULES: what a module is allowed to write, +// what a caller is allowed to see, and what the prune takes away. The three worth +// protecting are the ones that are easy to "simplify" into a leak: +// +// 1. a module writes only into its OWN active Teams, named by external id; +// 2. visibility defaults to `members` and is resolved from the session, never +// from a request parameter; +// 3. a hidden Team's feed does not answer a public caller. +const { test, beforeEach, afterEach } = require('node:test') +const assert = require('node:assert/strict') + +const activityDb = require('../src/model/teams/teamActivity.db') +const teamsDb = require('../src/model/teams/teams.db') +const access = require('../src/model/teams/teamAccess.model') +const settings = require('../src/model/settings/settings.model') +const activity = require('../src/model/teams/teamActivity.model') + +let store +const saved = new Map() + +function patch(mod, name, fn) { + if (!saved.has(mod)) saved.set(mod, new Map()) + if (!saved.get(mod).has(name)) saved.get(mod).set(name, mod[name]) + mod[name] = fn +} + +function restore() { + for (const [mod, names] of saved) for (const [name, fn] of names) mod[name] = fn + saved.clear() +} + +function stub() { + store = { + rows: [], + nextId: 1, + teams: [ + { id: 1, module_id: 'uo', external_id: 'g1', slug: 'the-guild', hidden: 0, status: 'active' }, + { id: 2, module_id: 'uo', external_id: 'g2', slug: 'hidden-guild', hidden: 1, status: 'active' }, + ], + allowed: new Set(), // userIds with member/grant access, keyed "teamId:userId" + } + + patch(teamsDb, 'findActive', async (moduleId, externalId) => + store.teams.find((t) => t.module_id === moduleId && t.external_id === externalId && t.status === 'active')) + patch(teamsDb, 'findBySlug', async (slug) => store.teams.find((t) => t.slug === slug)) + + patch(access, 'forumAccess', async (teamId, userId) => ({ + allowed: store.allowed.has(`${teamId}:${userId}`), + viaMembership: store.allowed.has(`${teamId}:${userId}`), + viaGrant: false, + isLeader: false, + })) + + patch(activityDb, 'insert', async (item) => { + if (item.dedupeKey && store.rows.some((r) => r.team_id === item.teamId && r.dedupe_key === item.dedupeKey)) { + return false // the unique index doing its job + } + store.rows.push({ + id: store.nextId++, + team_id: item.teamId, + source: item.source, + kind: item.kind, + summary: item.summary, + visibility: item.visibility, + actor_member_key: item.actorMemberKey, + actor_user_id: item.actorUserId, + payload: item.payload, + occurred_at: new Date(item.occurredAt), + dedupe_key: item.dedupeKey, + }) + return true + }) + + patch(activityDb, 'page', async (teamId, visibilities, { limit, offset }) => + store.rows + .filter((r) => r.team_id === teamId && visibilities.includes(r.visibility)) + .sort((a, b) => b.occurred_at - a.occurred_at || b.id - a.id) + .slice(offset, offset + limit)) + + patch(activityDb, 'count', async (teamId, visibilities) => + store.rows.filter((r) => r.team_id === teamId && visibilities.includes(r.visibility)).length) + + patch(activityDb, 'deleteOlderThan', async (days) => { + const cutoff = Date.now() - days * 86400_000 + const before = store.rows.length + store.rows = store.rows.filter((r) => r.occurred_at.getTime() >= cutoff) + return before - store.rows.length + }) + + patch(activityDb, 'overCap', async (cap) => { + const byTeam = new Map() + for (const r of store.rows) byTeam.set(r.team_id, (byTeam.get(r.team_id) || 0) + 1) + return [...byTeam].filter(([, n]) => n > cap).map(([team_id, n]) => ({ team_id, n })) + }) + + patch(activityDb, 'trimToCap', async (teamId, cap) => { + const mine = store.rows + .filter((r) => r.team_id === teamId) + .sort((a, b) => b.occurred_at - a.occurred_at || b.id - a.id) + const keep = new Set(mine.slice(0, cap).map((r) => r.id)) + const before = store.rows.length + store.rows = store.rows.filter((r) => r.team_id !== teamId || keep.has(r.id)) + return before - store.rows.length + }) + + patch(settings, 'get', async () => null) // defaults +} + +const item = (extra = {}) => ({ externalId: 'g1', kind: 'uo.thing', summary: 'A thing happened', ...extra }) + +beforeEach(stub) +afterEach(restore) + +// ── What a module may write ──────────────────────────────────────────────── + +test('a pushed item lands against the team its external id names', async () => { + const stored = await activity.push('uo', [item()]) + assert.equal(stored, 1) + assert.equal(store.rows[0].team_id, 1) + assert.equal(store.rows[0].source, 'uo') +}) + +test('a module cannot write into another module\'s team', async () => { + // 'other' owns no team with external id g1, so there is nothing to resolve — + // and no integer the module could have sent instead, which is the point of + // naming Teams by external id on this path. + const stored = await activity.push('other', [item()]) + assert.equal(stored, 0) + assert.equal(store.rows.length, 0) +}) + +test('an unknown external id is dropped rather than raised', async () => { + const stored = await activity.push('uo', [item({ externalId: 'nope' })]) + assert.equal(stored, 0) +}) + +test('visibility defaults to members, and an unknown value does not widen it', async () => { + await activity.push('uo', [item(), item({ visibility: 'everyone' }), item({ visibility: 'public' })]) + assert.deepEqual(store.rows.map((r) => r.visibility), ['members', 'members', 'public']) +}) + +test('an item with no kind or no summary is dropped, and the rest of the batch still lands', async () => { + const stored = await activity.push('uo', [item({ kind: '' }), item({ summary: ' ' }), item()]) + assert.equal(stored, 1) + assert.equal(store.rows.length, 1) +}) + +test('an over-long summary is truncated rather than losing the event', async () => { + await activity.push('uo', [item({ summary: 'x'.repeat(400) })]) + assert.equal(store.rows[0].summary.length, 255) +}) + +test('a replayed batch with dedupe keys stores each item once', async () => { + const batch = [item({ dedupeKey: 'champ:77' }), item({ dedupeKey: 'champ:78' })] + await activity.push('uo', batch) + await activity.push('uo', batch) // the sidecar reconnect backfill + assert.equal(store.rows.length, 2) +}) + +test('items without a dedupe key are never collapsed into each other', async () => { + await activity.push('uo', [item(), item()]) + assert.equal(store.rows.length, 2) +}) + +test('a push is never rejected for being malformed at the top level', async () => { + assert.equal(await activity.push('uo', null), 0) + assert.equal(await activity.push('uo', []), 0) +}) + +// ── What a caller may see ────────────────────────────────────────────────── + +test('an anonymous caller gets public items only, and is told the scope', async () => { + await activity.push('uo', [item({ visibility: 'public' }), item({ visibility: 'members' })]) + const feed = await activity.feedFor('the-guild', null) + assert.equal(feed.items.length, 1) + assert.equal(feed.items[0].visibility, 'public') + assert.equal(feed.scope, 'public') + // `total` is the caller's total, not the table's — otherwise paging lies. + assert.equal(feed.total, 1) +}) + +test('a member sees both, via the same resolver the forum uses', async () => { + await activity.push('uo', [item({ visibility: 'public' }), item({ visibility: 'members' })]) + store.allowed.add('1:7') + const feed = await activity.feedFor('the-guild', 7) + assert.equal(feed.items.length, 2) + assert.equal(feed.scope, 'members') +}) + +test('an authenticated non-member is exactly an anonymous caller here', async () => { + await activity.push('uo', [item({ visibility: 'members' })]) + const feed = await activity.feedFor('the-guild', 99) + assert.equal(feed.items.length, 0) + assert.equal(feed.scope, 'public') +}) + +test('a hidden team\'s feed does not answer the public, but does answer its members', async () => { + await activity.push('uo', [item({ externalId: 'g2', visibility: 'public' })]) + assert.equal(await activity.feedFor('hidden-guild', null), null) + + store.allowed.add('2:7') + const feed = await activity.feedFor('hidden-guild', 7) + assert.equal(feed.items.length, 1) +}) + +test('an unknown slug is not found rather than empty', async () => { + assert.equal(await activity.feedFor('no-such-team', null), null) +}) + +test('the rendered item carries the payload and never the actor identifiers', async () => { + await activity.push('uo', [item({ + visibility: 'public', payload: { serial: '0x77' }, actorMemberKey: '0x40012ab3', actorUserId: 7, + })]) + const feed = await activity.feedFor('the-guild', null) + assert.deepEqual(feed.items[0].payload, { serial: '0x77' }) + assert.equal('actorMemberKey' in feed.items[0], false) + assert.equal('actorUserId' in feed.items[0], false) +}) + +// ── Core's own items ─────────────────────────────────────────────────────── + +test('core writes as source=core and public', async () => { + await activity.logCore({ teamId: 1, kind: activity.CORE_KINDS.MEMBER_JOINED, summary: 'Aldric joined' }) + assert.equal(store.rows[0].source, 'core') + assert.equal(store.rows[0].visibility, 'public') +}) + +test('core refuses an item with nothing to say', async () => { + assert.equal(await activity.logCore({ teamId: 1, kind: 'core.x' }), false) + assert.equal(store.rows.length, 0) +}) + +// ── Retention ────────────────────────────────────────────────────────────── + +test('the prune drops rows past the age horizon', async () => { + const old = Date.now() - 100 * 86400_000 + await activity.push('uo', [item({ occurredAt: old }), item()]) + const res = await activity.prune() + assert.equal(res.days, activity.DEFAULT_RETAIN_DAYS) + assert.equal(res.byAge, 1) + assert.equal(store.rows.length, 1) +}) + +test('the prune trims a team back to the row cap, newest kept', async () => { + patch(settings, 'get', async (key) => (key === activity.CAP_KEY ? '3' : null)) + const base = Date.now() + for (let i = 0; i < 6; i++) { + // eslint-disable-next-line no-await-in-loop + await activity.push('uo', [item({ summary: `event ${i}`, occurredAt: base + i * 1000 })]) + } + const res = await activity.prune() + assert.equal(res.byCap, 3) + assert.deepEqual(store.rows.map((r) => r.summary), ['event 3', 'event 4', 'event 5']) +}) + +test('a zero or negative retention setting is rejected rather than emptying the feed', async () => { + patch(settings, 'get', async (key) => (key === activity.RETAIN_KEY ? '0' : null)) + await activity.push('uo', [item()]) + const res = await activity.prune() + assert.equal(res.days, activity.DEFAULT_RETAIN_DAYS) + assert.equal(store.rows.length, 1) +}) + +test('an unreadable settings table leaves the defaults standing', async () => { + patch(settings, 'get', async () => { throw new Error('pool down') }) + const res = await activity.retentionConfig() + assert.deepEqual(res, { days: activity.DEFAULT_RETAIN_DAYS, cap: activity.DEFAULT_ROW_CAP }) +}) diff --git a/server/test/teamSync.test.js b/server/test/teamSync.test.js index 614f986..26ea2aa 100644 --- a/server/test/teamSync.test.js +++ b/server/test/teamSync.test.js @@ -12,6 +12,7 @@ const assert = require('node:assert/strict') const registries = require('../src/modules/registries') const teamsDb = require('../src/model/teams/teams.db') const moderation = require('../src/model/teams/teamModeration.model') +const activity = require('../src/model/teams/teamActivity.model') const settings = require('../src/model/settings/settings.model') const teamSync = require('../src/model/teams/teamSync.model') @@ -113,6 +114,16 @@ function stubDb() { patch(teamsDb, 'memberKeys', async (teamId) => [...membersOf(teamId).values()].filter((m) => m.status === 'active').map((m) => m.member_key)) + // The sync reads the full rows, not just the keys: the activity feed needs each + // changing member's display name and PRIOR is_leader, both of which the upsert + // is about to overwrite. Stubbing this is not optional — an unstubbed seam here + // reaches the real pool, and the symptom is the suite hanging on dead-pool + // retries rather than failing (see test/_setup.js). + patch(teamsDb, 'membersByTeam', async (teamId, { includeDeparted = false } = {}) => + [...membersOf(teamId).values()] + .filter((m) => includeDeparted || m.status === 'active') + .map((m) => ({ ...m }))) + patch(teamsDb, 'upsertMember', async (m) => { const existing = membersOf(m.teamId).get(m.memberKey) membersOf(m.teamId).set(m.memberKey, { @@ -182,6 +193,16 @@ function stubDb() { return { hidden: false } }) patch(moderation, 'rescreen', async () => 0) + + // The activity feed is its own unit (teamActivity.test.js); here it is captured + // so the reconciler's side of §4.2 can be asserted without a database. Stubbing + // the MODEL rather than the db layer keeps these tests about which items the + // sync decides to emit, which is the reconciler's half of the contract. + store.activity = [] + patch(activity, 'logCore', async (item) => { + store.activity.push(item) + return true + }) } // A provider whose answers the test controls. Defaults are authoritative and @@ -802,3 +823,155 @@ test('start() is inert with no provider registered', async () => { await teamSync.start() assert.equal(store.teams.length, 0) }) + +// ── Core's own activity items (§4.2) ─────────────────────────────────────── +// +// The reconciler's half of the feed: which items it DECIDES to emit. The feed's +// own rules — visibility, dedupe, retention — live in teamActivity.test.js. + +const kinds = () => store.activity.map((a) => a.kind) +const summaries = () => store.activity.map((a) => a.summary) + +const withMembers = (members, leaders = []) => ({ + getTeams: async () => ({ ok: true, teams: [team('g1', 'A')] }), + getTeamMembers: async () => ({ ok: true, members }), + getTeamLeaders: async () => ({ ok: true, leaders }), +}) + +// Re-provide between runs: the tests above establish that a provider registers +// once, so a second answer means a fresh registration. +async function resync(overrides, reason) { + registries._reset() + provide(overrides) + return teamSync.reconcileNow(reason) +} + +test('the FIRST roster emits nothing — an import is not 155 people joining', async () => { + provide(withMembers([member('0x1'), member('0x2')])) + await teamSync.reconcileNow('setup') + assert.equal(activeMembers(1).length, 2, 'the members did land') + assert.deepEqual(store.activity, [], 'and none of them was announced') +}) + +test('a member arriving after the first roster is announced', async () => { + provide(withMembers([member('0x1')])) + await teamSync.reconcileNow('setup') + + await resync(withMembers([member('0x1'), member('0x2', { displayName: 'Brenna' })]), 'test') + + assert.deepEqual(kinds(), ['core.member.joined']) + assert.deepEqual(summaries(), ['Brenna joined']) + assert.equal(store.activity[0].actorMemberKey, '0x2') +}) + +test('a member who leaves is announced by the name core last knew them by', async () => { + provide(withMembers([member('0x1'), member('0x2', { displayName: 'Brenna' })])) + await teamSync.reconcileNow('setup') + + await resync(withMembers([member('0x1')]), 'test') + + // The module no longer mentions them at all, so the display name can only come + // from the row core is about to depart — which is why the sync reads the ROWS + // before the upsert rather than just the keys. + assert.deepEqual(kinds(), ['core.member.left']) + assert.deepEqual(summaries(), ['Brenna left']) +}) + +test('an INCOMPLETE answer announces no departures, because it removed none', async () => { + provide(withMembers([member('0x1'), member('0x2')])) + await teamSync.reconcileNow('setup') + + await resync({ + getTeams: async () => ({ ok: true, teams: [team('g1', 'A')] }), + getTeamMembers: async () => ({ ok: true, complete: false, members: [member('0x1')] }), + getTeamLeaders: async () => ({ ok: true, leaders: [] }), + }, 'partial') + + assert.equal(activeMembers(1).length, 2, 'nobody was removed') + assert.deepEqual(store.activity, [], 'so nobody is announced as leaving') +}) + +test('promotion and demotion are announced; unchanged leadership is not', async () => { + provide(withMembers([member('0x1'), member('0x2')], ['0x1'])) + await teamSync.reconcileNow('setup') + + await resync(withMembers([member('0x1'), member('0x2')], ['0x2']), 'test') + + assert.deepEqual(kinds(), ['core.leader.changed', 'core.leader.changed']) + assert.deepEqual(summaries().sort(), ['0x1 stepped down as a leader', '0x2 became a leader']) +}) + +test('a member who joins already a leader is announced once, as joining', async () => { + provide(withMembers([member('0x1')], ['0x1'])) + await teamSync.reconcileNow('setup') + + await resync(withMembers([member('0x1'), member('0x2')], ['0x1', '0x2']), 'test') + + assert.deepEqual(kinds(), ['core.member.joined'], 'never a non-leader here to be promoted from') +}) + +test('a departing leader is announced as leaving, not as stepping down', async () => { + provide(withMembers([member('0x1'), member('0x2')], ['0x1', '0x2'])) + await teamSync.reconcileNow('setup') + + await resync(withMembers([member('0x1')], ['0x1']), 'test') + + assert.deepEqual(kinds(), ['core.member.left'], 'one event, not two') +}) + +test('a refused leadership answer announces nothing — it demoted nobody', async () => { + provide(withMembers([member('0x1')], ['0x1'])) + await teamSync.reconcileNow('setup') + + await resync({ + getTeams: async () => ({ ok: true, teams: [team('g1', 'A')] }), + getTeamMembers: async () => ({ ok: true, members: [member('0x1')] }), + getTeamLeaders: async () => ({ ok: false, reason: 'unavailable' }), + }, 'test') + + assert.deepEqual(store.activity, []) + assert.equal(activeMembers(1)[0].is_leader, 1, 'and left the stored value alone') +}) + +test('a rename is announced on the successor, naming the old name', async () => { + provide({ + getTeams: async () => ({ ok: true, teams: [team('g1', 'The Silver Hand')] }), + getTeamMembers: async () => ({ ok: true, members: [] }), + getTeamLeaders: async () => ({ ok: true, leaders: [] }), + }) + await teamSync.reconcileNow('setup') + + await resync({ + getTeams: async () => ({ ok: true, teams: [team('g1', 'The Golden Hand')] }), + getTeamMembers: async () => ({ ok: true, members: [] }), + getTeamLeaders: async () => ({ ok: true, leaders: [] }), + }, 'rename') + + const renames = store.activity.filter((a) => a.kind === 'core.team.renamed') + assert.equal(renames.length, 1) + assert.equal(renames[0].summary, 'Renamed from The Silver Hand') + // The SUCCESSOR row, not the archived one: the archived row is a read-only + // record of what came before, and the reader is looking at the live page. + const successor = store.teams.find((t) => t.status === 'active') + assert.equal(renames[0].teamId, successor.id) +}) + +test('a member with no display name is announced without leaking the member key', async () => { + provide(withMembers([member('0x1')])) + await teamSync.reconcileNow('setup') + + await resync(withMembers([member('0x1'), member('0x2', { displayName: null })]), 'test') + + assert.deepEqual(summaries(), ['A member joined']) +}) + +test('a feed write that fails never fails the sync', async () => { + patch(activity, 'logCore', async () => { throw new Error('table gone') }) + provide(withMembers([member('0x1')])) + await teamSync.reconcileNow('setup') + + const result = await resync(withMembers([member('0x1'), member('0x2')]), 'test') + + assert.equal(result.ok, true) + assert.equal(activeMembers(1).length, 2, 'the roster is the source of truth and it applied') +}) -- 2.49.1 From 03631d7d40cf54895f1c8fb5bf6085352dda13c9 Mon Sep 17 00:00:00 2001 From: wtclaude Date: Mon, 17 Aug 2026 20:15:36 -0500 Subject: [PATCH 2/5] feat(teams): the roster's audience projection, and optionalAuth to resolve it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TEAMS.md §3.3, as the eighth member of MODULE_API 1.6.0 — amended in place per the org lead, on the rule Protocol 4 was given in phase 2: a contract owes a bump only once it has landed on `main`. Two questions meet on the roster and they belong to different owners. WHICH ROWS a viewer may see is the module's, because the audience rungs and their configuration live there and core does not know what a rung is. WHAT A ROW LOOKS LIKE stays core's. So `projectRoster` answers with member KEYS, not rows. §3.3 said rows, and rows would let a module widen what is published — handing back a `userId` core had withheld — leaving core's field guarantee resting on every module's good behaviour. Core asks which rows and re-normalises the answer through its own public shape, so a module can narrow and cannot widen. "The module declines" needed splitting before it could be implemented. No module at all and a module whose rungs could not be consulted are opposite situations: the first withholds nothing and must serve the roster whole, the second must serve none of it. The refusal carries `projects`, and only `projects: true` fails closed. Without the split, bare core serves an empty roster on every Team page. This is also the first public route whose CONTENT depends on identity, which needed a middleware core did not have. `attachSession` only decodes a token, so a banned account, a password change or a logout would have kept working against the private half of a feed until the JWT expired. `optionalAuth` runs requireAuth's full database re-validation and, on any failure, continues ANONYMOUSLY rather than rejecting — a caller whose session is no longer good sees the public view, which is what they are entitled to. `GET /public/teams/:slug/activity` lands here for the same reason: §2.11's route table had no activity endpoint though §4.3 describes a filtered feed. Paged, with the visibility resolved from the session and never from a parameter. Co-Authored-By: Claude --- server/src/auth/session.middleware.js | 39 ++++++ server/src/model/teams/teamProvider.js | 57 ++++++++ server/src/model/teams/teams.model.js | 63 ++++++++- server/src/modules/registries.js | 21 ++- .../src/router/v1/public/teams.controller.js | 38 +++++- server/src/router/v1/public/teams.router.js | 25 +++- server/swagger/swagger.js | 41 ++++++ server/test/teamProvider.test.js | 87 +++++++++++++ server/test/teamRoster.test.js | 122 ++++++++++++++++++ 9 files changed, 486 insertions(+), 7 deletions(-) create mode 100644 server/test/teamRoster.test.js diff --git a/server/src/auth/session.middleware.js b/server/src/auth/session.middleware.js index 1171858..93ca03e 100644 --- a/server/src/auth/session.middleware.js +++ b/server/src/auth/session.middleware.js @@ -79,6 +79,44 @@ async function requireAuth(req, res, next) { } } +// Best-effort AUTHENTICATION, as opposed to attachSession's best-effort decode. +// +// For a PUBLIC route whose content — not merely its presentation — depends on who +// is asking. The Team activity feed is the first: `public` items go to everyone +// and `members` items only to members and forum-granted users (TEAMS.md §4.3), so +// an anonymous caller must be served, not rejected, and an authenticated one must +// be identified properly. +// +// "Properly" is why this is not attachSession. That one decodes the token and +// stops, which is right for reading back your own session but wrong here: a +// banned account, a password change, or a logout would all keep working against +// the private half of the feed until the JWT expired. This runs the same +// database re-validation requireAuth does — status, cutoff, revocation — and on +// any failure continues ANONYMOUSLY rather than 401ing. A caller whose session is +// no longer good sees the public feed, which is exactly what they are entitled to. +// +// A database error also degrades to anonymous. On a public route the safe +// direction is to serve less, and 500ing a page because a session lookup failed +// would take the whole Team page down for callers who never sent a token. +async function optionalAuth(req, res, next) { + const session = sessionService.validateSession(req) + if (!session) return next() + try { + const user = await users.getById(session.userId) + if (!user) return next() + if (user.status && user.status !== 'active') return next() + if (isBeforeCutoff(session, user.tokens_valid_after)) return next() + if (await sessionService.isSessionRevoked(session.sessionId)) return next() + + req.user = user + req.session = session + req.authMethod = session.authMethod + } catch (err) { + log.warn('optionalAuth: continuing anonymously', { message: err.message }) + } + return next() +} + // Gate middleware factory: allow only the listed roles. Assumes requireAuth ran // first so req.user is populated. Use for admin-only endpoints (users, site // mode, settings) so a lower-privilege editor cannot reach them. @@ -91,6 +129,7 @@ function requireRole(...roles) { module.exports = { attachSession, + optionalAuth, requireAuth, requireRole, } diff --git a/server/src/model/teams/teamProvider.js b/server/src/model/teams/teamProvider.js index 945f983..b2253b7 100644 --- a/server/src/model/teams/teamProvider.js +++ b/server/src/model/teams/teamProvider.js @@ -163,10 +163,66 @@ function normaliseLeaders(answer) { return { ok: true, leaders } } +/** + * `{ ok, members: [memberKey] }` — WHICH rows the module permits this viewer. + * + * Deliberately a set of keys rather than a set of rows. Core already holds the + * rows and knows their public shape; asking the module for rows back would let a + * module widen what is published — re-adding a `userId` or a `memberKey` that + * §3.2 says is never published — and core's field guarantee would then rest on + * every module's good behaviour rather than on core. So the module answers the + * question it actually owns (who may be seen at this rung) and core keeps the + * question it owns (what a member row looks like in public). + */ +function normaliseVisibleKeys(answer) { + if (!Array.isArray(answer.members)) return fail('projectRoster() answered ok with no members array') + const keys = [] + for (const raw of answer.members) { + const key = str(raw) + if (!key) return fail('a projectRoster() entry is not a member key') + if (!keys.includes(key)) keys.push(key) + } + return { ok: true, members: keys } +} + const getTeams = () => call('getTeams', normaliseTeams) const getTeamMembers = (externalId) => call('getTeamMembers', normaliseMembers, externalId) const getTeamLeaders = (externalId) => call('getTeamLeaders', normaliseLeaders, externalId) +/** + * Ask the module which roster rows this viewer may see (§3.3). + * + * The per-audience projection is the module's because the visibility framework + * and its rung configuration are module-owned (§10.5) — core does not know what a + * rung is. Core supplies the roster and a description of the viewer; the module + * returns the member keys it permits. + * + * **"No audience model" and "could not answer" are different, and the caller must + * be able to tell them apart** — so the refusal carries `projects`. + * + * `projects: false` — no provider is registered, or the registered one does not + * implement `projectRoster`. There is no rung system to consult and nothing + * is being withheld; the roster is served at core's public shape. This is why + * the member is OPTIONAL: bare core, and a module with no audience model of + * its own, both render exactly the page core writes. + * + * `projects: true` — the module HAS an audience model and core could not reach + * it (refused, threw, timed out, answered malformed). Here the caller must + * fail CLOSED, because "leave it alone" would mean publishing the very rows + * the rungs exist to withhold. This is the one place in the Team subsystem + * where unavailability is not staleness: everywhere else a refused call + * leaves data alone, and doing that to a *visibility* question is a leak. + */ +async function projectRoster(externalId, members, viewer) { + const provider = registries.registeredTeamProvider() + if (!provider) return { ...fail('no team provider is registered'), projects: false } + if (typeof provider.projectRoster !== 'function') { + return { ...fail('provider does not project rosters'), projects: false } + } + const answer = await call('projectRoster', normaliseVisibleKeys, externalId, members, viewer) + return answer.ok ? answer : { ...answer, projects: true } +} + /** Which module is authoritative, or null. The reconciler keys sync state on it. */ const providerModuleId = () => { const provider = registries.registeredTeamProvider() @@ -177,6 +233,7 @@ module.exports = { getTeams, getTeamMembers, getTeamLeaders, + projectRoster, providerModuleId, CALL_TIMEOUT_MS, } diff --git a/server/src/model/teams/teams.model.js b/server/src/model/teams/teams.model.js index 7f09dae..c25eff2 100644 --- a/server/src/model/teams/teams.model.js +++ b/server/src/model/teams/teams.model.js @@ -156,6 +156,16 @@ async function listPublic({ limit = 50, offset = 0 } = {}) { teams: visible.slice(offset, offset + limit).map(publicTeam), total: visible.length, ...sync, + // What the `teams` nav feature flag resolves from (§3.5). True if a provider + // is registered OR any Team exists — the second half matters because Team + // rows outlive the module that filled them, and hiding the nav entry the + // moment a module is uninstalled would make every existing Team page + // unreachable from the site while still answering by URL. + // + // False only when there is nothing and no prospect of anything, which is + // exactly the bare-core case the flag exists for: a link to a permanently + // empty page is worse than no link. + enabled: Boolean(sync.configured) || visible.length > 0, } } @@ -174,6 +184,19 @@ async function getPublic(slug) { const successor = row.succeeded_by ? await teamsDb.findById(row.succeeded_by) : null return { ...publicTeam(row), + // The three props the `team.overview` extension slot is declared with + // (§3.4). A module's slot component runs in the browser and has to know + // WHICH Team it is looking at, in its own vocabulary — `slug` is core's name + // for it and resolves nothing on the module's side. + // + // On this route only, deliberately: the index has no slot and would + // otherwise publish a module-internal identifier per row for nothing. None + // of the three names a person — they are a core row id, a game-side group + // id and a module name, and the identifiers §3.2 withholds (member keys, + // site account ids) are not among them. + id: row.id, + externalId: row.external_id, + moduleId: row.module_id, ...sync, successor: successor && !successor.hidden ? { slug: successor.slug, name: successor.display_name_override || successor.name } @@ -181,14 +204,50 @@ async function getPublic(slug) { } } -async function rosterPublic(slug) { +/** + * A Team's roster, projected for the caller's audience rung (§3.3). + * + * The ROW filter is the module's: it owns the visibility framework and its + * configuration (§10.5), and core does not know what a rung is. The FIELD shape + * stays core's — every row that survives goes through `publicMember`, which + * withholds the member key and the user id whatever the module answers. So a + * module can narrow what is published and cannot widen it, and core's "neither is + * published" guarantee does not rest on every module's good behaviour. + * + * **A module that HAS a rung system and cannot answer withholds the roster.** That + * is the one Team call where a refusal is not staleness: leaving a visibility + * answer "alone" would publish the very rows the rungs exist to withhold. A + * deployment with no module, or one whose module does not project at all, is a + * different case entirely — nothing is being withheld there, so the roster is + * served whole at core's public shape (`projects: false`). + */ +async function rosterPublic(slug, viewer = null) { const row = await teamsDb.findBySlug(slug) if (!row || row.hidden) return null const [members, sync] = await Promise.all([ access.rosterWithOverrides(row.id), syncStatus(), ]) - return { members: members.map(publicMember), ...sync, rosterSyncedAt: row.roster_synced_at } + + // The module gets the rows as it supplied them — this is its own data coming + // home — plus who is asking, which is all a rung decision needs. + const answer = await teamProvider.projectRoster(row.external_id, members, viewer) + let visible + if (answer.ok) visible = members.filter((m) => answer.members.includes(m.member_key)) + else if (answer.projects) visible = [] // fail closed: it has rungs and we could not ask + else visible = members // nothing to fail closed ABOUT + + return { + members: visible.map(publicMember), + ...sync, + rosterSyncedAt: row.roster_synced_at, + // Stated rather than implied. An empty roster has three quite different + // causes — a Team with no members, a rung that shows none, and a module that + // could not be asked — and a page that cannot tell them apart will report the + // last one as the first. + projected: answer.ok, + ...(answer.ok || !answer.projects ? {} : { projectionUnavailable: true }), + } } // ── Player ───────────────────────────────────────────────────────────────── diff --git a/server/src/modules/registries.js b/server/src/modules/registries.js index 7165f8e..e0fa24c 100644 --- a/server/src/modules/registries.js +++ b/server/src/modules/registries.js @@ -243,11 +243,22 @@ function checkLegShape(entry) { return { leg, label: label || leg, dispatch, classify } } -// All three methods are REQUIRED, with no optional half. A provider that could -// list Teams but not their members would leave core holding Teams it can never +// Three methods are REQUIRED, with no optional half. A provider that could list +// Teams but not their members would leave core holding Teams it can never // populate, and the reconciler has no sensible behaviour for that — it is not the // same as a call that fails, which is staleness and already handled (§2.4). A // module unable to answer one of the three answers `{ ok: false }` at call time. +// +// `projectRoster` is the fourth and is OPTIONAL (TEAMS.md §3.3): it expresses an +// audience model, and a module with no rung system of its own has no opinion to +// express. Omitting it means core serves rosters at its own public shape; +// implementing it means core fails CLOSED when the call cannot be made, so this +// is a member to add deliberately rather than by habit. +// +// The copy is explicit rather than a spread: this object is what core calls, so +// anything not named here is not part of the contract and must not survive +// registration. A method that silently rode along would look implemented from the +// module's side and be invisible from core's. function checkTeamProviderShape(entry) { const provider = entry || {} const out = {} @@ -257,6 +268,12 @@ function checkTeamProviderShape(entry) { } out[name] = provider[name] } + if (provider.projectRoster !== undefined) { + if (typeof provider.projectRoster !== 'function') { + throw new Error('registerTeamProvider: projectRoster must be a function if present') + } + out.projectRoster = provider.projectRoster + } return out } diff --git a/server/src/router/v1/public/teams.controller.js b/server/src/router/v1/public/teams.controller.js index 701cc9b..e92da83 100644 --- a/server/src/router/v1/public/teams.controller.js +++ b/server/src/router/v1/public/teams.controller.js @@ -5,6 +5,7 @@ // marked stale, because that is what the projection is for. const teams = require('../../../model/teams/teams.model') +const teamActivity = require('../../../model/teams/teamActivity.model') const log = require('../../../utils/logger')('teams') @@ -35,9 +36,18 @@ async function getTeam(req, res) { } } +/** + * The roster, projected for whoever is asking (§3.3). + * + * The viewer is described to the module rather than handed over: it gets the + * caller's id and role, which is what a rung decision turns on, and not the user + * row — a module has `ctx.users.getById` if it needs more, and passing the whole + * record here would make every column of `users` part of this contract. + */ async function getRoster(req, res) { try { - const roster = await teams.rosterPublic(req.params.slug) + const viewer = req.user ? { userId: req.user.id, role: req.user.role } : null + const roster = await teams.rosterPublic(req.params.slug, viewer) if (!roster) return res.status(404).json({ message: 'Team not found' }) return res.json(roster) } catch (err) { @@ -45,4 +55,28 @@ async function getRoster(req, res) { } } -module.exports = { listTeams, getTeam, getRoster } +/** + * A Team's activity feed (§4.3). + * + * The only handler in this tier that reads `req.user`, and it reads nothing else + * from the caller about what they may see: `limit` and `offset` are page + * controls, and the visibility filter is resolved from the session alone. A + * request parameter naming its own visibility is the bug the ENUM exists to + * prevent, so there is deliberately no way to ask for one. + * + * The cap is 100 rather than the index's 200 — every row carries a summary and an + * opaque payload, so a page of these is much larger than a page of Teams. + */ +async function getActivity(req, res) { + try { + const limit = Math.min(Math.max(Number.parseInt(req.query.limit, 10) || 50, 1), 100) + const offset = Math.max(Number.parseInt(req.query.offset, 10) || 0, 0) + const feed = await teamActivity.feedFor(req.params.slug, req.user ? req.user.id : null, { limit, offset }) + if (!feed) return res.status(404).json({ message: 'Team not found' }) + return res.json(feed) + } catch (err) { + return fail(res, err, 'activity') + } +} + +module.exports = { listTeams, getTeam, getRoster, getActivity } diff --git a/server/src/router/v1/public/teams.router.js b/server/src/router/v1/public/teams.router.js index 3b17d25..71f12ef 100644 --- a/server/src/router/v1/public/teams.router.js +++ b/server/src/router/v1/public/teams.router.js @@ -12,6 +12,7 @@ const express = require('express') const ctrl = require('./teams.controller') const siteMode = require('../../../middleware/siteMode') +const { optionalAuth } = require('../../../auth/session.middleware') const teamsRouter = express.Router() @@ -43,12 +44,34 @@ teamsRouter.get( '/:slug/members', // #swagger.tags = ['Public · Teams'] // #swagger.summary = 'Get a Team roster' - // #swagger.description = 'In-game display names only. A member key is a game-internal identifier and a user id names a site account; neither is published. `linked` answers whether a character has an account behind it without saying which.' + // #swagger.description = 'In-game display names only. A member key is a game-internal identifier and a user id names a site account; neither is published, whatever the module’s projection answers. `linked` answers whether a character has an account behind it without saying which. WHICH rows appear is the module’s audience projection; sending a session is optional and may widen it.' // #swagger.parameters['slug'] = { in: 'path', required: true, schema: { type: 'string' }, description: 'The Team slug.' } + // #swagger.security = [{}, { "cookieAuth": [] }, { "bearerAuth": [] }] /* #swagger.responses[200] = { description: 'The roster, with sync freshness', content: { "application/json": { schema: { $ref: "#/components/schemas/PublicTeamRoster" } } } } */ /* #swagger.responses[404] = { description: 'No such Team, or it is hidden', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ siteMode, + optionalAuth, ctrl.getRoster, ) +// The one route in this tier that reads the caller's identity. `optionalAuth` +// serves anonymous callers rather than rejecting them, and identifies an +// authenticated one properly enough that a banned or logged-out account drops +// back to the public half of the feed at once (TEAMS.md §4.3). +teamsRouter.get( + '/:slug/activity', + // #swagger.tags = ['Public · Teams'] + // #swagger.summary = 'A Team’s activity feed, filtered to what the caller may see' + // #swagger.description = 'Items are `public` or `members`. Anyone who can see the Team gets the public ones; members and forum-granted users also get the members-only ones, and the response says which via `scope` so a client can render "some items are hidden" rather than presenting a filtered feed as the whole one. Sending a session is optional.' + // #swagger.parameters['slug'] = { in: 'path', required: true, schema: { type: 'string' }, description: 'The Team slug.' } + // #swagger.parameters['limit'] = { in: 'query', required: false, schema: { type: 'integer' }, description: 'Page size, max 100 (default 50).' } + // #swagger.parameters['offset'] = { in: 'query', required: false, schema: { type: 'integer' }, description: 'Rows to skip (default 0).' } + // #swagger.security = [{}, { "cookieAuth": [] }, { "bearerAuth": [] }] + /* #swagger.responses[200] = { description: 'One page of the feed', content: { "application/json": { schema: { $ref: "#/components/schemas/PublicTeamActivity" } } } } */ + /* #swagger.responses[404] = { description: 'No such Team, or it is hidden from this caller', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ + siteMode, + optionalAuth, + ctrl.getActivity, +) + module.exports = teamsRouter diff --git a/server/swagger/swagger.js b/server/swagger/swagger.js index 03ebf40..47731ee 100644 --- a/server/swagger/swagger.js +++ b/server/swagger/swagger.js @@ -979,6 +979,12 @@ const doc = { properties: { teams: { type: 'array', items: { $ref: '#/components/schemas/PublicTeam' } }, total: { type: 'integer', example: 12 }, + enabled: { + type: 'boolean', + description: + 'Whether this deployment has Teams at all — a provider is registered, or Teams exist from one that since went away. The `teams` nav feature flag resolves from this; false means bare core, where a Teams link would lead to a permanently empty page.', + example: true, + }, }, }, PublicTeamMember: { @@ -999,6 +1005,41 @@ const doc = { properties: { members: { type: 'array', items: { $ref: '#/components/schemas/PublicTeamMember' } }, rosterSyncedAt: { type: 'string', format: 'date-time', nullable: true }, + projected: { + type: 'boolean', + description: + 'Whether the module applied its own audience projection to this roster. False means the module declined or does not project, and the roster was served at core’s public shape — never the full one.', + example: true, + }, + }, + }, + PublicTeamActivityItem: { + type: 'object', + description: + '`summary` is already-rendered text supplied by whoever pushed the item; core never composes one. `kind` and `payload` are opaque to core — only the module’s `team.overview` slot renders anything richer than the text.', + properties: { + id: { type: 'integer', example: 4821 }, + source: { type: 'string', description: '`core` or a module id.', example: 'uo' }, + kind: { type: 'string', example: 'uo.champion.completed' }, + summary: { type: 'string', example: 'Completed Champion Neira' }, + visibility: { type: 'string', enum: ['public', 'members'] }, + occurredAt: { type: 'string', format: 'date-time' }, + payload: { type: 'object', nullable: true, additionalProperties: true }, + }, + }, + PublicTeamActivity: { + type: 'object', + properties: { + items: { type: 'array', items: { $ref: '#/components/schemas/PublicTeamActivityItem' } }, + total: { type: 'integer', description: 'Matching rows for THIS caller’s visibility, so paging is honest.', example: 137 }, + limit: { type: 'integer', example: 50 }, + offset: { type: 'integer', example: 0 }, + scope: { + type: 'string', + enum: ['public', 'members'], + description: + 'Which visibilities this caller received. `public` means members-only items were withheld — render that fact rather than presenting a filtered feed as the whole one.', + }, }, }, PlayerTeamList: { diff --git a/server/test/teamProvider.test.js b/server/test/teamProvider.test.js index 75bdd5b..f160d6b 100644 --- a/server/test/teamProvider.test.js +++ b/server/test/teamProvider.test.js @@ -287,3 +287,90 @@ test('a hung call does not hold the process open until its deadline', async () = test('the budget is the documented ten seconds', () => { assert.equal(teamProvider.CALL_TIMEOUT_MS, 10_000) }) + +// ── projectRoster: the optional fourth member (§3.3) ─────────────────────── +// +// The one Team call where a refusal must NOT be treated as staleness. Every test +// below exists because the obvious implementation — reuse `call()` and serve the +// roster when it fails — silently publishes the rows the rungs exist to withhold. + +const rows = [{ member_key: '0x1' }, { member_key: '0x2' }] + +test('projectRoster is optional: a provider without it registers fine', () => { + const api = registries.stage('uo') + assert.doesNotThrow(() => api.registerTeamProvider(ok())) +}) + +test('a non-function projectRoster is rejected at registration, not at call time', () => { + const api = registries.stage('uo') + assert.throws( + () => api.registerTeamProvider({ ...ok(), projectRoster: 'yes please' }), + /projectRoster must be a function/, + ) +}) + +test('an unregistered method cannot ride along into the provider core calls', () => { + register('uo', { ...ok(), somethingElse: async () => 'hi' }) + assert.equal(registries.registeredTeamProvider().somethingElse, undefined) +}) + +test('no provider at all is projects:false — nothing is being withheld', async () => { + const answer = await teamProvider.projectRoster('g1', rows, null) + assert.equal(answer.ok, false) + assert.equal(answer.projects, false) +}) + +test('a provider that does not project is projects:false, not a failure to fear', async () => { + register('uo', ok()) + const answer = await teamProvider.projectRoster('g1', rows, null) + assert.equal(answer.ok, false) + assert.equal(answer.projects, false) +}) + +test('a provider that HAS projectRoster and refuses is projects:true — the caller must fail closed', async () => { + register('uo', { ...ok(), projectRoster: async () => ({ ok: false, reason: 'atlas not loaded' }) }) + const answer = await teamProvider.projectRoster('g1', rows, null) + assert.equal(answer.ok, false) + assert.equal(answer.projects, true) + assert.equal(answer.reason, 'atlas not loaded') +}) + +test('a projectRoster that throws is projects:true as well — a bug is not permission', async () => { + register('uo', { ...ok(), projectRoster: async () => { throw new Error('boom') } }) + const answer = await teamProvider.projectRoster('g1', rows, null) + assert.equal(answer.projects, true) +}) + +test('the module receives the rows and the viewer, and answers with member keys', async () => { + let seen + register('uo', { + ...ok(), + projectRoster: async (externalId, members, viewer) => { + seen = { externalId, members, viewer } + return { ok: true, members: ['0x2'] } + }, + }) + const answer = await teamProvider.projectRoster('g1', rows, { userId: 7, role: 'player' }) + assert.deepEqual(seen.members, rows) + assert.deepEqual(seen.viewer, { userId: 7, role: 'player' }) + assert.equal(seen.externalId, 'g1') + assert.deepEqual(answer.members, ['0x2']) +}) + +test('a malformed key list is a refusal, so the caller fails closed rather than serving garbage', async () => { + for (const bad of [{ ok: true }, { ok: true, members: ['ok', ''] }, { ok: true, members: 'all' }]) { + // eslint-disable-next-line no-await-in-loop + register('uo', { ...ok(), projectRoster: async () => bad }) + // eslint-disable-next-line no-await-in-loop + const answer = await teamProvider.projectRoster('g1', rows, null) + assert.equal(answer.ok, false, JSON.stringify(bad)) + assert.equal(answer.projects, true) + registries._reset() + } +}) + +test('duplicate keys are collapsed', async () => { + register('uo', { ...ok(), projectRoster: async () => ({ ok: true, members: ['0x1', '0x1', '0x2'] }) }) + const answer = await teamProvider.projectRoster('g1', rows, null) + assert.deepEqual(answer.members, ['0x1', '0x2']) +}) diff --git a/server/test/teamRoster.test.js b/server/test/teamRoster.test.js new file mode 100644 index 0000000..a0b45ba --- /dev/null +++ b/server/test/teamRoster.test.js @@ -0,0 +1,122 @@ +// The roster read and its audience projection (docs/website/TEAMS.md §3.2, §3.3). +// +// Two questions meet here and the file exists to keep them apart: +// +// WHICH ROWS is the module's — it owns the visibility framework and its rung +// configuration, and core does not know what a rung is. +// WHAT A ROW is core's — the member key and the user id are never published, +// LOOKS LIKE whatever the module answers. +// +// The dangerous simplification is to let the module return rows instead of keys: +// core's field guarantee would then rest on every module's good behaviour rather +// than on core, and one module re-adding a `userId` would publish site accounts +// against in-game characters on a public page. +const { test, beforeEach, afterEach } = require('node:test') +const assert = require('node:assert/strict') + +const teamsDb = require('../src/model/teams/teams.db') +const access = require('../src/model/teams/teamAccess.model') +const teamProvider = require('../src/model/teams/teamProvider') +const teamSync = require('../src/model/teams/teamSync.model') +const teams = require('../src/model/teams/teams.model') + +const saved = new Map() + +function patch(mod, name, fn) { + if (!saved.has(mod)) saved.set(mod, new Map()) + if (!saved.get(mod).has(name)) saved.get(mod).set(name, mod[name]) + mod[name] = fn +} + +function restore() { + for (const [mod, names] of saved) for (const [name, fn] of names) mod[name] = fn + saved.clear() +} + +const ROWS = [ + { member_key: '0x1', display_name: 'Aldric', user_id: 7, is_leader: 1, rank_label: 'Leader', online: 1 }, + { member_key: '0x2', display_name: 'Brenna', user_id: null, is_leader: 0, rank_label: null, online: 0 }, + { member_key: '0x3', display_name: 'Cadfael', user_id: 9, is_leader: 0, rank_label: null, online: 0 }, +] + +beforeEach(() => { + patch(teamsDb, 'findBySlug', async (slug) => + (slug === 'the-guild' + ? { id: 1, external_id: 'g1', slug, hidden: 0, status: 'active', roster_synced_at: null } + : undefined)) + patch(access, 'rosterWithOverrides', async () => ROWS.map((r) => ({ ...r }))) + // syncStatus() reads sync state and the poll interval; neither is what this + // file is about, and both would otherwise reach the pool. + patch(teamProvider, 'providerModuleId', () => null) + patch(teamSync, 'intervalSeconds', async () => 900) +}) +afterEach(restore) + +test('with no module projecting, the whole roster is served at core\'s public shape', async () => { + patch(teamProvider, 'projectRoster', async () => ({ ok: false, projects: false, reason: 'no provider' })) + const roster = await teams.rosterPublic('the-guild', null) + assert.equal(roster.members.length, 3) + assert.equal(roster.projected, false) + assert.equal(roster.projectionUnavailable, undefined, 'nothing was withheld, so nothing to report') +}) + +test('the module chooses which rows a viewer sees', async () => { + patch(teamProvider, 'projectRoster', async () => ({ ok: true, members: ['0x2'] })) + const roster = await teams.rosterPublic('the-guild', null) + assert.deepEqual(roster.members.map((m) => m.displayName), ['Brenna']) + assert.equal(roster.projected, true) +}) + +test('a module that projects but cannot answer withholds the roster — it does not serve it', async () => { + // The whole point. "Leave it alone" is right for a roster SYNC and wrong for a + // visibility question: it would publish exactly what the rungs withhold. + patch(teamProvider, 'projectRoster', async () => ({ ok: false, projects: true, reason: 'sidecar down' })) + const roster = await teams.rosterPublic('the-guild', null) + assert.deepEqual(roster.members, []) + assert.equal(roster.projected, false) + assert.equal(roster.projectionUnavailable, true, 'an empty roster must be distinguishable from a silent one') +}) + +test('the module cannot widen the published fields, only narrow the rows', async () => { + // A module answering with keys it was given still yields core's shape. There is + // no answer it can give that puts a member key or a user id on a public page. + patch(teamProvider, 'projectRoster', async () => ({ ok: true, members: ['0x1', '0x2', '0x3'] })) + const roster = await teams.rosterPublic('the-guild', null) + for (const member of roster.members) { + assert.deepEqual( + Object.keys(member).sort(), + ['displayName', 'isLeader', 'linked', 'online', 'rankLabel'], + 'the public member shape is core\'s and is closed', + ) + } + assert.deepEqual(roster.members.map((m) => m.linked), [true, false, true]) +}) + +test('a key the module invents matches nothing rather than adding a row', async () => { + patch(teamProvider, 'projectRoster', async () => ({ ok: true, members: ['0x1', '0xNOPE'] })) + const roster = await teams.rosterPublic('the-guild', null) + assert.equal(roster.members.length, 1) +}) + +test('the viewer is described to the module, not handed over', async () => { + let seen + patch(teamProvider, 'projectRoster', async (externalId, members, viewer) => { + seen = viewer + return { ok: true, members: members.map((m) => m.member_key) } + }) + await teams.rosterPublic('the-guild', { userId: 7, role: 'player' }) + assert.deepEqual(seen, { userId: 7, role: 'player' }) +}) + +test('an unknown slug is not found, and the module is never consulted about it', async () => { + let called = false + patch(teamProvider, 'projectRoster', async () => { called = true; return { ok: true, members: [] } }) + assert.equal(await teams.rosterPublic('no-such-team', null), null) + assert.equal(called, false) +}) + +test('a hidden team\'s roster does not answer publicly at all', async () => { + patch(teamsDb, 'findBySlug', async () => ({ id: 1, external_id: 'g1', slug: 'x', hidden: 1, status: 'active' })) + patch(teamProvider, 'projectRoster', async () => ({ ok: true, members: ['0x1'] })) + assert.equal(await teams.rosterPublic('x', null), null) +}) -- 2.49.1 From 8f4aff69463f67087f0a783b1c57f55d99a4b60f Mon Sep 17 00:00:00 2001 From: wtclaude Date: Mon, 17 Aug 2026 20:15:54 -0500 Subject: [PATCH 3/5] feat(teams): the public Team pages, the two slots and the nav flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TEAMS.md §3.1–§3.5. Four core pages — the index, a Team's overview, its full roster and the player portal's "My Teams" — plus the two extension slots a module adds to them, and the nav rows that lead there. These are CORE routes, not module ones. A Team is a core platform entity that a module merely populates, so the whole experience renders on bare core; a module adds to these pages rather than supplying them. `team.member.row` is declared with `{ displayName, isLeader, linked }` and not §3.4's `{ memberKey, userId, displayName }`. The two documents contradict each other and §3.2 is the one that is a security rule: a slot component runs in the browser, so those props can only reach it by publishing a game-internal identifier and a site account id in every public roster response, for every visitor, module installed or not. Recorded as an amendment. The presentation logic is split into lib/teams.js with its own tests, following lib/teamAdmin.js, because these pages have to state differences that read as bugs unless they are worded deliberately: - "37 members · 21 linked" — the gap is information (a character with no site account behind it), and the header says what each number IS rather than showing both and hoping; - an empty roster has three unrelated causes — nobody in the Team, a rung that shows nobody, and a module that could not be asked — and reporting the last as the first is a statement about the game that happens to be false; - a stale projection says how old it is rather than presenting itself as current. `teams` is the first CORE nav row to carry a `feature` since the shard rows left with the module cutover, and it brings core's own feature provider back with it. It gates on whether this deployment has Teams AT ALL, not on who is looking — Team pages are public and the server gates them. It fails open, so an unknown answer shows the link: a Teams link leading somewhere empty is a far cheaper mistake than a Team page nobody can find. Co-Authored-By: Claude --- client/src/App.jsx | 16 ++ client/src/api/client.js | 23 +++ client/src/components/SiteHeader.jsx | 5 + client/src/lib/teams.js | 151 ++++++++++++++++ client/src/main.jsx | 23 ++- client/src/modules/coreFeatures.js | 56 ++++++ .../src/routes/player/PlayerPortalLayout.jsx | 8 + client/src/routes/player/PlayerTeams.jsx | 67 ++++++++ client/src/routes/public/Team.jsx | 126 ++++++++++++++ client/src/routes/public/TeamRoster.jsx | 120 +++++++++++++ client/src/routes/public/Teams.jsx | 107 ++++++++++++ client/test/teams.test.js | 162 ++++++++++++++++++ 12 files changed, 861 insertions(+), 3 deletions(-) create mode 100644 client/src/lib/teams.js create mode 100644 client/src/modules/coreFeatures.js create mode 100644 client/src/routes/player/PlayerTeams.jsx create mode 100644 client/src/routes/public/Team.jsx create mode 100644 client/src/routes/public/TeamRoster.jsx create mode 100644 client/src/routes/public/Teams.jsx create mode 100644 client/test/teams.test.js diff --git a/client/src/App.jsx b/client/src/App.jsx index 1750361..6353cc5 100644 --- a/client/src/App.jsx +++ b/client/src/App.jsx @@ -18,6 +18,9 @@ import Newsletter from './routes/public/Newsletter.jsx' import NewsletterIssue from './routes/public/NewsletterIssue.jsx' import About from './routes/public/About.jsx' import Status from './routes/public/Status.jsx' +import Teams from './routes/public/Teams.jsx' +import Team from './routes/public/Team.jsx' +import TeamRoster from './routes/public/TeamRoster.jsx' import Wiki from './routes/wiki/Wiki.jsx' import WikiArticle from './routes/wiki/WikiArticle.jsx' import CmsPage from './routes/public/CmsPage.jsx' @@ -57,6 +60,7 @@ import AcceptInvite from './routes/player/AcceptInvite.jsx' import PlayerPortalLayout, { PlayerIndex } from './routes/player/PlayerPortalLayout.jsx' import PlayerAccount from './routes/player/PlayerAccount.jsx' import PlayerAppeals from './routes/player/PlayerAppeals.jsx' +import PlayerTeams from './routes/player/PlayerTeams.jsx' export default function App() { return ( @@ -93,6 +97,13 @@ export default function App() { } /> } /> } /> + {/* Teams (TEAMS.md §3.1). Core routes, not module ones: a Team is a + core platform entity that a module merely populates, so these + render on bare core too. `/teams` is declared before `/:slug` + below for the same reason every named route is. */} + } /> + } /> + } /> {/* Installed modules' public pages, namespaced `//…` — the registry prefixes the segment, so a module cannot spell its way out of it (docs/website/MODULE_API.md §3.3). Declared before the @@ -216,6 +227,11 @@ export default function App() { } /> } /> } /> + {/* Core's own page under the /player prefix, unlike the module + pages below it. Open to any authenticated account, not role + 'player': staff are a superset of players and a moderator is in + guilds too — RequirePlayer above already draws that line. */} + } /> {/* Installed modules' player-portal pages, at /player//…. This group's own routes are absolute (its layout route has no path), so the prefix is written here rather than inherited — the one diff --git a/client/src/api/client.js b/client/src/api/client.js index 64359c4..bd19f4d 100644 --- a/client/src/api/client.js +++ b/client/src/api/client.js @@ -133,6 +133,29 @@ export const api = { return req(`/public/wiki${withQs(s)}`) }, wikiCategories: () => req('/public/wiki/categories'), + + // ----- Teams (TEAMS.md §2.11, §3.1) ----- + // + // Public reads, but three of them behave differently for a signed-in caller and + // the session rides along on the cookie the shared `req` already sends: the + // roster may widen with the module's audience projection, and the activity feed + // adds members-only items. None of them REQUIRES a session. + teams: (opts = {}) => { + const qs = new URLSearchParams() + if (opts.limit != null) qs.set('limit', String(opts.limit)) + if (opts.offset != null) qs.set('offset', String(opts.offset)) + return req(`/public/teams${withQs(qs.toString())}`) + }, + team: (slug) => req(`/public/teams/${encodeURIComponent(slug)}`), + teamRoster: (slug) => req(`/public/teams/${encodeURIComponent(slug)}/members`), + teamActivity: (slug, opts = {}) => { + const qs = new URLSearchParams() + if (opts.limit != null) qs.set('limit', String(opts.limit)) + if (opts.offset != null) qs.set('offset', String(opts.offset)) + return req(`/public/teams/${encodeURIComponent(slug)}/activity${withQs(qs.toString())}`) + }, + myTeams: () => req('/player/teams'), + myTeamAccess: (slug) => req(`/player/teams/${encodeURIComponent(slug)}/access`), wikiTags: () => req('/public/wiki/tags'), wikiPage: (slug) => req(`/public/wiki/${slug}`), // CMS pages (block-based). Published-only for the public; a draft-preview link diff --git a/client/src/components/SiteHeader.jsx b/client/src/components/SiteHeader.jsx index 84706f0..064536d 100644 --- a/client/src/components/SiteHeader.jsx +++ b/client/src/components/SiteHeader.jsx @@ -28,6 +28,11 @@ import { useFeatureGate } from '../modules/features.jsx' export const NAV = [ { label: 'Home', to: '/', end: true }, { label: 'News', to: '/site/news' }, + // The first CORE row to carry a `feature` since the shard rows left in slice 3 + // (TEAMS.md §3.5). It is answered by core's own provider (main.jsx) and gates + // on whether this deployment has Teams at all, not on who is looking — Team + // pages are public. Fails open, so an unknown answer shows the link. + { label: 'Teams', to: '/teams', feature: 'teams' }, { label: 'Screenshots', to: '/site/screenshots' }, { label: 'Five on Friday', to: '/site/five-on-friday' }, { label: 'Newsletter', to: '/site/newsletter' }, diff --git a/client/src/lib/teams.js b/client/src/lib/teams.js new file mode 100644 index 0000000..e73a208 --- /dev/null +++ b/client/src/lib/teams.js @@ -0,0 +1,151 @@ +// What the public Team pages SAY, separated from how they render +// (docs/website/TEAMS.md §3.2, §3.3, §4.3). +// +// Plain JS with tests, following lib/teamAdmin.js. It is worth splitting here for +// the same reason it was there: these pages have to state differences that look +// like bugs unless they are worded deliberately. A roster header reading +// "37 members · 21 linked" is information; the same numbers with no explanation +// is a support ticket. And an empty roster has three unrelated causes — a Team +// with nobody in it, an audience rung that shows nobody, and a module that could +// not be asked — which is logic, not markup. + +/** How the roster describes a row's relationship to a site account (§3.2). */ +export const LINK_STATE = { linked: 'linked', unlinked: 'unlinked' } + +export function linkStateOf(member) { + return member && member.linked ? LINK_STATE.linked : LINK_STATE.unlinked +} + +/** + * The roster header line. + * + * The gap between the two numbers is the surfaced divergence Part 2 asks for: it + * must read as information rather than as a discrepancy, which is why the line + * says what each number IS instead of showing them side by side and hoping. + * + * `guests` is phase 4's forum grants and is omitted while there are none, so the + * line does not carry a permanent zero for a feature that has not shipped. + */ +export function rosterSummary({ members = 0, linked = 0, guests = 0 } = {}) { + const parts = [`${members} ${members === 1 ? 'member' : 'members'}`, `${linked} linked`] + if (guests > 0) parts.push(`${guests} forum ${guests === 1 ? 'guest' : 'guests'}`) + return parts.join(' · ') +} + +const MINUTE = 60_000 +const HOUR = 60 * MINUTE +const DAY = 24 * HOUR + +/** "just now" / "14 minutes ago" / "3 hours ago" / "2 days ago". */ +export function relativeTime(when, now = Date.now()) { + if (!when) return null + const ms = now - new Date(when).getTime() + if (!Number.isFinite(ms)) return null + if (ms < MINUTE) return 'just now' + if (ms < HOUR) { + const n = Math.floor(ms / MINUTE) + return `${n} ${n === 1 ? 'minute' : 'minutes'} ago` + } + if (ms < DAY) { + const n = Math.floor(ms / HOUR) + return `${n} ${n === 1 ? 'hour' : 'hours'} ago` + } + const n = Math.floor(ms / DAY) + return `${n} ${n === 1 ? 'day' : 'days'} ago` +} + +/** + * How a public page describes the projection's freshness (§2.4). + * + * Distinct from `teamAdmin.freshnessOf`, which is worded for an operator + * debugging a sync. A visitor needs one sentence about whether what they are + * looking at is current, and specifically must never be shown an unconfirmed + * empty projection as though it were a confirmed empty shard. + */ +export function freshnessNote(sync = {}, now = Date.now()) { + // Nothing supplies Teams here, so there is nothing to be stale ABOUT. A + // deployment with no game module is not a broken one. + if (!sync.configured) return null + if (!sync.lastSyncAt) return { tone: 'warn', text: 'Not yet confirmed against the game.' } + const ago = relativeTime(sync.lastSyncAt, now) + if (sync.stale) return { tone: 'warn', text: `Last confirmed ${ago} — the game may have moved on.` } + return { tone: 'idle', text: `Last confirmed ${ago}.` } +} + +/** + * Why a roster is empty, in the viewer's terms. + * + * Returns null when it is not empty. The three causes are genuinely different and + * reporting the wrong one is the failure this function exists to prevent: telling + * someone a guild has no members when in fact the module could not be asked is a + * statement about the game that happens to be false. + */ +export function emptyRosterReason(roster = {}) { + const members = roster.members || [] + if (members.length) return null + if (roster.projectionUnavailable) { + return 'The roster cannot be shown right now — the game module could not be reached.' + } + if (roster.configured && !roster.lastSyncAt) { + return 'This roster has not been confirmed against the game yet.' + } + return 'Nobody is in this Team.' +} + +/** + * Group feed items into days, newest first, preserving order within a day (§4.3). + * + * Keyed by local calendar date rather than by a UTC slice: "yesterday" is a + * property of where the reader is sitting, and a shard's evening raid landing at + * 00:30 UTC belongs on the day the players experienced it. + */ +export function groupByDay(items = [], locale = undefined) { + const days = [] + const byKey = new Map() + for (const item of items) { + const date = new Date(item.occurredAt) + if (Number.isNaN(date.getTime())) continue + const key = `${date.getFullYear()}-${date.getMonth()}-${date.getDate()}` + if (!byKey.has(key)) { + const day = { + key, + label: date.toLocaleDateString(locale, { year: 'numeric', month: 'long', day: 'numeric' }), + items: [], + } + byKey.set(key, day) + days.push(day) + } + byKey.get(key).items.push(item) + } + return days +} + +/** + * What to say under a feed that has been filtered. + * + * Only when there is something to say: a caller who saw everything is told + * nothing, and an anonymous caller is invited to sign in rather than simply + * informed that items exist which they cannot have. + */ +export function activityScopeNote(feed = {}, signedIn = false) { + if (feed.scope !== 'public') return null + return signedIn + ? 'Some entries are visible to members of this Team only.' + : 'Sign in as a member of this Team to see its members-only entries.' +} + +/** Sort for the index: most members first, then alphabetically. */ +export function sortTeams(teams = []) { + return [...teams].sort( + (a, b) => (b.memberCount || 0) - (a.memberCount || 0) || String(a.name).localeCompare(String(b.name)), + ) +} + +/** The index's search, over the two things a visitor knows a Team by. */ +export function filterTeams(teams = [], query = '') { + const q = query.trim().toLowerCase() + if (!q) return teams + return teams.filter( + (t) => String(t.name || '').toLowerCase().includes(q) || String(t.abbr || '').toLowerCase().includes(q), + ) +} diff --git a/client/src/main.jsx b/client/src/main.jsx index c2c3101..8ab9ff7 100644 --- a/client/src/main.jsx +++ b/client/src/main.jsx @@ -3,7 +3,8 @@ import { createRoot } from 'react-dom/client' import { BrowserRouter } from 'react-router-dom' import App from './App.jsx' import { publishSharedDependencies } from './modules/shared.js' -import { declareSlot } from './modules/registry.js' +import { declareSlot, registerFeatureProvider } from './modules/registry.js' +import { useCoreFlags } from './modules/coreFeatures.js' import './styles/theme.css' // Publish window.__rg BEFORE rendering and before any module chunk evaluates. @@ -18,8 +19,12 @@ publishSharedDependencies() // and namespace `uo`, so that the seam was exercised by real content from the // day it was built. That prediction paid out exactly as written: the extraction // deleted the registration and the hook it named, and SiteHeader was not touched. -// There is nothing for core to register now — no core nav row carries a -// `feature` — and the filter is a correct no-op until a module supplies one. +// +// Teams put a core row back on the seam. `feature: 'teams'` on the three Team nav +// rows resolves against owner id `core` (featureGate.js: a row with no `moduleId` +// belongs to core), and this is the provider that answers it — hiding the rows on +// a deployment that has no Teams at all, and failing open everywhere else. +registerFeatureProvider('core', 'core', useCoreFlags) // ── Extension slots (MODULE_API.md §3.7) ─────────────────────────────────── // @@ -50,6 +55,18 @@ declareSlot('admin.users.detail') // over `onDone`. With the slot unfilled the invitee goes straight to the portal, // which is what core's own code did whenever the flag was off. declareSlot('player.invite.accepted') +// The two Team slots (TEAMS.md §3.4, MODULE_API 1.6.0). Both named for a place: +// `team.overview` is the spot under the counts on a Team page, not "where the +// game puts guild stats", and `team.member.row` is the trailing cell of a roster +// row. Core renders the whole Team experience with both unfilled — the pages are +// core's, and a module adds to them rather than supplying them. +// +// `team.overview` is where a live "online now" strip belongs: core's online count +// is the durable floor refreshed at the reconcile interval (§3.3), and a module +// that already holds a live presence feed can render the current number here +// without core acquiring an SSE stack to do it. +declareSlot('team.overview') +declareSlot('team.member.row') // Core filled the first two itself until slice 3, with the components that were // inline in SiteFooter.jsx and UserDetail.jsx. Both are gone: the module fills diff --git a/client/src/modules/coreFeatures.js b/client/src/modules/coreFeatures.js new file mode 100644 index 0000000..c797157 --- /dev/null +++ b/client/src/modules/coreFeatures.js @@ -0,0 +1,56 @@ +import { useEffect, useState } from 'react' + +// Core's own feature provider (TEAMS.md §3.5, MODULE_API.md §3.3). +// +// Core registered one here until the module cutover, under owner id `core` and +// namespace `uo`, and it left with the shard rows. This brings the seam back with +// content that is genuinely core's: `teams` gates the Teams nav rows, and Teams +// are a core platform entity that a module merely populates. +// +// **What the flag actually answers is "does this deployment have Teams at all".** +// Not "may this viewer see them" — Team pages are public (§0.7) and the server +// gates them. On bare core, with no module supplying a Team provider and no rows +// left behind by one, `/teams` is a permanently empty page and a link to it is +// worse than no link. That is the whole job. +// +// It fails OPEN, like every other answer in this seam: while the request is in +// flight, and on any error, the hook returns `null`, which `buildFeatureGate` +// reads as "we do not know yet" and SHOWS the row. The page itself is the gate. +// The one thing a UI mistake must never do here is hide a surface from someone +// entitled to it — and a Teams link that leads somewhere empty is a far cheaper +// mistake than a Team page nobody can find. + +// `limit=1` because only `enabled` is wanted. The endpoint answers it whatever +// the page size, and asking for the default fifty would pull a roster's worth of +// counts into a nav decision. +const TEAMS_URL = '/api/v1/public/teams?limit=1' + +/** + * The hook core registers. Returns a Set-like of visible flags, or `null` while + * the answer is unknown. + * + * Fetched once per mount rather than subscribed: whether a deployment has Teams + * changes when a module is installed, which is a restart, not a session event. + */ +export function useCoreFlags() { + const [flags, setFlags] = useState(null) + + useEffect(() => { + let active = true + fetch(TEAMS_URL, { credentials: 'same-origin' }) + .then((res) => (res.ok ? res.json() : null)) + .then((body) => { + if (!active) return + // A body that does not carry `enabled` is an older server or a shape + // change, and both are "unknown" rather than "no". + if (!body || typeof body.enabled !== 'boolean') return + setFlags(new Set(body.enabled ? ['teams'] : [])) + }) + .catch(() => {}) // stays null: unknown shows the row + return () => { active = false } + }, []) + + return flags +} + +export default useCoreFlags diff --git a/client/src/routes/player/PlayerPortalLayout.jsx b/client/src/routes/player/PlayerPortalLayout.jsx index 12e9153..cd1c537 100644 --- a/client/src/routes/player/PlayerPortalLayout.jsx +++ b/client/src/routes/player/PlayerPortalLayout.jsx @@ -35,6 +35,7 @@ function Icon({ children, size = 16 }) { } const IconGear = () => const IconShield = () => +const IconTeams = () => // Exported because Admin -> Navigation edits this list. It stays declared here; // the editor may only relabel, reorder and hide what it finds (§7). No CORE row @@ -46,6 +47,12 @@ const IconShield = () =>