From 2fa4d87a400bbad171f87a01e0f9228d6e85d6d3 Mon Sep 17 00:00:00 2001 From: wtclaude Date: Mon, 17 Aug 2026 12:57:26 -0500 Subject: [PATCH] feat(shard): ingest guild rosters and departures (protocol 4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Protocol 2 gave the guild board a member *count* and nothing else, so the Guilds page could say a guild had 155 members but never who they were, and findGuildForActor deliberately answered only for leaders because membership for rank-and-file was not in the feed at all. Protocol 4 puts it there. `shard_guild_members` holds one row per member per guild, keyed on (guild_id, serial). `guild.roster` replaces a guild's rows; `guild.leave` removes one. A guild.remove now clears the membership too, so a disbanded guild does not leave orphaned rows behind. The chunking needs explaining. A roster over the shard's per-frame cap arrives as several frames carrying seq/more/total. The sidecar reassembles them for its own GET /guilds board, but the live WebSocket feed and the /history backfill both carry the individual frames — so this ingest sees them unreassembled. It copes without buffering, because a table expresses what the sidecar's single JSON column could not: the frame carrying seq 0 clears the guild first, and every frame then upserts its own rows. Upsert rather than insert because the /history backfill replays stored frames on every reconnect, and a redelivery has to be a no-op rather than a duplicate-key error. The cost is a sub-second window during a multi-frame update where the table holds part of a roster; buffering to close it would duplicate the sidecar's reassembly for a projection that is already only as fresh as a 60s sweep. On visibility: both kinds are mapped to the existing `guilds` feature. Without that mapping rule 2 fails an unmapped kind closed to admin-only, which would have quietly kept rosters off the public page forever. Mapping them is safe because a roster is the first frame carrying locked fields inside an ARRAY of actors rather than one nested actor, and the projection walker already recurses into arrays and matches acct/webId by suffix — so a member's account name is stripped below admin by exactly the rule that already strips guild.leader.acct. There is a test for that specifically, because the difference is a public page listing character names versus one publishing 150 account names. `acct`/`web_id` are still stored, since that is what lets a linked member be matched to a site user; they are just never projected below admin. guild.leave is appended to the event log, as the departure counterpart to guild.join and for the same reason — it is what a "so-and-so left" feed reads. guild.roster stays out: it is board state like guild.update, and it is the one fat frame on the wire, so logging it would put a full membership snapshot into shard_events on every membership change. The PUBLIC_KINDS guard test caught the addition, which is what it is for; its expected set now carries a v4 group alongside the v3 one. Refs: docs/website/TEAMS.md Part 12 Phase 1 Co-Authored-By: Claude --- .gitea/workflows/pr-checks.yml | 9 ++- server/db/purge.sql | 1 + server/db/schema.sql | 28 ++++++++ server/model/shardState/shardState.db.js | 36 ++++++++++ server/model/shardState/shardState.model.js | 79 ++++++++++++++++++++- server/test/shardIngest.guildRoster.test.js | 75 +++++++++++++++++++ server/test/shardVisibility.test.js | 56 ++++++++++++++- server/utils/shardIngest.js | 16 +++++ server/utils/shardVisibility.js | 7 ++ 9 files changed, 302 insertions(+), 5 deletions(-) create mode 100644 server/test/shardIngest.guildRoster.test.js diff --git a/.gitea/workflows/pr-checks.yml b/.gitea/workflows/pr-checks.yml index 052c666..08cb264 100644 --- a/.gitea/workflows/pr-checks.yml +++ b/.gitea/workflows/pr-checks.yml @@ -61,12 +61,19 @@ # # Runner: the shared self-hosted `ubuntu-latest` runner. These jobs need only # Node — no Docker socket, no database. +# +# Scope note: `edge` is gated as well as `main`. Multi-phase work lands there +# first, so gating only the `main` hop would run these checks for the first time +# at the cutover — the one moment a red build is most expensive to discover. This +# is the same call `RunicGateway/installer` made for the same reason, and it was +# taken here after a nine-PR Android workstream landed on an ungated `edge` with +# no CI at all. Adding a branch to the `branches:` list is the whole change. name: PR Checks on: pull_request: - branches: [main] + branches: [main, edge] # A newer push to the same PR cancels the in-flight run. concurrency: diff --git a/server/db/purge.sql b/server/db/purge.sql index 7b6cc48..86d5679 100644 --- a/server/db/purge.sql +++ b/server/db/purge.sql @@ -45,6 +45,7 @@ DROP TABLE IF EXISTS `shard_ruleset`; DROP TABLE IF EXISTS `shard_presence`; DROP TABLE IF EXISTS `shard_governor_terms`; DROP TABLE IF EXISTS `shard_governors`; +DROP TABLE IF EXISTS `shard_guild_members`; DROP TABLE IF EXISTS `shard_guilds`; DROP TABLE IF EXISTS `shard_pages`; DROP TABLE IF EXISTS `shard_champs`; diff --git a/server/db/schema.sql b/server/db/schema.sql index ae263e4..7d56898 100644 --- a/server/db/schema.sql +++ b/server/db/schema.sql @@ -220,6 +220,34 @@ CREATE TABLE IF NOT EXISTS shard_guilds ( INDEX idx_shard_guilds_name (name) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; +-- Guild membership (Protocol 4). One row per member per guild, replaced on +-- guild.roster and thinned by guild.leave. Protocol 2 could only say HOW MANY +-- members a guild had, so this table has no pre-4 equivalent and the Guilds page +-- could show a count but never a roster. +-- +-- `acct` / `web_id` are the site-identity fields and are stored because the +-- sidecar forwards them; they are NOT public. shardVisibility locks any key that +-- is or ends in acct/webId to `admin` and recurses into arrays, so a projected +-- roster loses them below that rung — storing them here is what lets a linked +-- member be matched to a site user at all. +-- +-- A roster over the shard's per-frame cap arrives in several frames, so rows are +-- keyed on (guild_id, serial) and the frame carrying seq 0 clears the guild first; +-- see upsertGuildRoster. +CREATE TABLE IF NOT EXISTS shard_guild_members ( + guild_id INT NOT NULL, + serial VARCHAR(20) NOT NULL, -- in-game mobile serial, "0x1F5" + name VARCHAR(120) NULL, + acct VARCHAR(120) NULL, -- absent for a mobile with no account + web_id INT NULL, -- set only when the account is linked + is_player TINYINT(1) NOT NULL DEFAULT 1, + t BIGINT NULL, -- roster event time, epoch ms + updated_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, + PRIMARY KEY (guild_id, serial), + INDEX idx_shard_guild_members_acct (acct), + INDEX idx_shard_guild_members_web (web_id) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; + -- Town-governor board (Protocol 2.0, City Loyalty). One row per city, upserted on -- city.update (full-state, emitted only on change; there is no remove event since -- the set of cities is fixed). governor / governorElect are actor objects diff --git a/server/model/shardState/shardState.db.js b/server/model/shardState/shardState.db.js index 5bdacfe..3c0233c 100644 --- a/server/model/shardState/shardState.db.js +++ b/server/model/shardState/shardState.db.js @@ -171,6 +171,37 @@ const removeGuild = (id) => query('DELETE FROM shard_guilds WHERE id = ?', [id]) const clearGuilds = () => query('DELETE FROM shard_guilds') const listGuilds = () => query(`SELECT ${GUILD_COLS} FROM shard_guilds ORDER BY name ASC`) +// ── Guild membership (Protocol 4) ────────────────────────────────────────── +const MEMBER_COLS = 'guild_id, serial, name, acct, web_id, is_player, t' + +// Upsert rather than plain insert: a roster frame can be redelivered (the /history +// backfill replays stored frames on every reconnect), and a redelivery must be a +// no-op rather than a duplicate-key error. +const upsertGuildMembers = (rows) => { + if (!rows.length) return Promise.resolve() + const values = rows.map(() => '(?, ?, ?, ?, ?, ?, ?)').join(', ') + const params = rows.flatMap((r) => [r.guild_id, r.serial, r.name, r.acct, r.web_id, r.is_player, r.t]) + return query( + `INSERT INTO shard_guild_members (${MEMBER_COLS}) VALUES ${values} + ON DUPLICATE KEY UPDATE name = VALUES(name), acct = VALUES(acct), + web_id = VALUES(web_id), is_player = VALUES(is_player), t = VALUES(t)`, + params, + ) +} + +const clearGuildMembers = (guildId) => + query('DELETE FROM shard_guild_members WHERE guild_id = ?', [guildId]) + +const removeGuildMember = (guildId, serial) => + query('DELETE FROM shard_guild_members WHERE guild_id = ? AND serial = ?', [guildId, serial]) + +const clearAllGuildMembers = () => query('DELETE FROM shard_guild_members') + +const listGuildMembers = (guildId) => + query(`SELECT ${MEMBER_COLS} FROM shard_guild_members WHERE guild_id = ? ORDER BY name ASC`, [ + guildId, + ]) + // The guild an actor LEADS — matched on the current board (leader_serial or the // linked leader_acct), so it reflects live state. Guild MEMBERSHIP for non-leaders // is not modelled (the board carries only counts + leader), so we don't guess it. @@ -342,6 +373,11 @@ module.exports = { removeGuild, clearGuilds, listGuilds, + upsertGuildMembers, + clearGuildMembers, + removeGuildMember, + clearAllGuildMembers, + listGuildMembers, findGuildLedByActor, listGuildsLedByAccounts, upsertGovernor, diff --git a/server/model/shardState/shardState.model.js b/server/model/shardState/shardState.model.js index 10a3ba2..afde59e 100644 --- a/server/model/shardState/shardState.model.js +++ b/server/model/shardState/shardState.model.js @@ -340,8 +340,80 @@ async function upsertGuild(ev) { }) } -const removeGuild = (id) => (id == null ? Promise.resolve() : db.removeGuild(id)) -const clearGuilds = () => db.clearGuilds() +const removeGuild = async (id) => { + if (id == null) return + await db.removeGuild(id) + await db.clearGuildMembers(id) +} +const clearGuilds = async () => { + await db.clearGuilds() + await db.clearAllGuildMembers() +} + +// ── Guild membership (Protocol 4) ────────────────────────────────────────── +// Apply one guild.roster frame. +// +// A roster larger than the shard's per-frame cap arrives as several frames +// carrying seq/more/total. The sidecar reassembles them for its OWN board, but the +// live WebSocket feed and the /history backfill both carry the individual frames, +// so this ingest sees them unreassembled and has to cope. +// +// It copes without buffering, because a table can express what a single JSON column +// could not: the frame carrying seq 0 clears the guild first and every frame then +// upserts its own rows. Rows are keyed on (guild_id, serial), so a redelivered frame +// — the /history backfill replays stored frames on every reconnect — is idempotent +// rather than a duplicate-key error. +// +// The cost is a brief window during a multi-frame update where the table holds part +// of a roster. That is acceptable for a projection that is already only as fresh as +// a 60s sweep, and the frames arrive back-to-back in one burst; buffering to close +// it would duplicate the sidecar's reassembly for a sub-second inconsistency. +async function upsertGuildRoster(ev) { + if (!ev || ev.id == null) return + + const seq = Number.isFinite(ev.seq) ? ev.seq : 0 + const members = Array.isArray(ev.members) ? ev.members : [] + + // seq 0 begins a roster and supersedes whatever was held for this guild. + if (seq === 0) await db.clearGuildMembers(ev.id) + + const rows = members + .filter((m) => m && m.serial) + .map((m) => ({ + guild_id: ev.id, + serial: m.serial, + name: m.name ?? null, + acct: m.acct ?? null, + web_id: Number.isFinite(m.webId) ? m.webId : null, + is_player: m.player ? 1 : 0, + t: Number.isFinite(ev.t) ? ev.t : null, + })) + + await db.upsertGuildMembers(rows) +} + +// A single departure (guild.leave). Advisory: the shard re-emits the full roster +// whenever the member set changes, so the table would converge on the next frame +// even if this were dropped. Applying it makes the change visible immediately +// instead of at the end of the sweep that produced it. +async function removeGuildMember(ev) { + if (!ev || ev.id == null || !ev.who) return + await db.removeGuildMember(ev.id, ev.who) +} + +// The membership roster for one guild, in the wire shape the projection expects +// (an array of actor objects), so shardVisibility strips acct/webId by the same +// rule it applies to guild.leader. +async function listGuildMembers(guildId) { + const rows = await db.listGuildMembers(guildId) + return rows.map((r) => ({ + serial: r.serial, + name: r.name, + ...(r.acct == null ? {} : { acct: r.acct }), + ...(r.web_id == null ? {} : { webId: r.web_id }), + player: !!r.is_player, + })) +} function shapeGuild(r) { const payload = typeof r.payload === 'string' ? safeJson(r.payload) : r.payload @@ -620,6 +692,9 @@ module.exports = { removeGuild, clearGuilds, listGuilds, + upsertGuildRoster, + removeGuildMember, + listGuildMembers, replaceGuilds, findGuildForActor, listGuildsLedForAccounts, diff --git a/server/test/shardIngest.guildRoster.test.js b/server/test/shardIngest.guildRoster.test.js new file mode 100644 index 0000000..9981480 --- /dev/null +++ b/server/test/shardIngest.guildRoster.test.js @@ -0,0 +1,75 @@ +// Protocol 4 membership routing: guild.roster (board state, possibly chunked) and +// guild.leave (a real-time departure, logged like its guild.join counterpart). +const { test, beforeEach } = require('node:test') +const assert = require('node:assert/strict') + +const shardIngest = require('../utils/shardIngest') + +function makeDeps() { + const calls = { roster: [], memberRemove: [], appended: [], broadcast: [] } + const noop = async () => {} + return { + calls, + shardEvents: { append: async (row) => { calls.appended.push(row); return true } }, + shardState: { + upsertGuildRoster: async (ev) => { calls.roster.push(ev) }, + removeGuildMember: async (ev) => { calls.memberRemove.push(ev) }, + upsertGuild: noop, removeGuild: noop, + clearOnline: noop, upsertOnline: noop, setOffline: noop, + addEconomySample: noop, + }, + shardLinks: { removeByAccount: noop }, + uoLinkConfig: { recordStatus: noop }, + broadcast: (ev) => { calls.broadcast.push(ev) }, + pushDispatch: async () => {}, + log: { warn() {}, info() {}, error() {} }, + } +} + +beforeEach(() => shardIngest.reset()) + +test('guild.roster routes to upsertGuildRoster and is NOT logged', async () => { + // It is board state like guild.update, and the one fat frame on the wire — + // logging it would put a full membership snapshot in shard_events on every + // membership change. + const deps = makeDeps() + const r = await shardIngest.ingest( + { kind: 'guild.roster', id: 7, seq: 0, more: false, total: 2, + members: [{ serial: '0x1', name: 'Ada' }, { serial: '0x2', name: 'Bo' }], t: 1 }, + deps, + ) + + assert.equal(deps.calls.roster.length, 1) + assert.equal(deps.calls.roster[0].id, 7) + assert.equal(deps.calls.roster[0].members.length, 2) + assert.equal(r.logged, false) +}) + +test('every frame of a chunked roster reaches the model, seq intact', async () => { + // The sidecar reassembles for its own board, but the live feed and the /history + // backfill both carry individual frames — so the model must see each one with its + // seq, which is what tells it whether to clear the guild first. + const deps = makeDeps() + + for (const [seq, more, serial] of [[0, true, '0x1'], [1, true, '0x2'], [2, false, '0x3']]) { + await shardIngest.ingest( + { kind: 'guild.roster', id: 7, seq, more, total: 3, members: [{ serial, name: serial }], t: 1 }, + deps, + ) + } + + assert.deepEqual(deps.calls.roster.map((e) => e.seq), [0, 1, 2]) + assert.deepEqual(deps.calls.roster.map((e) => e.more), [true, true, false]) +}) + +test('guild.leave is logged and broadcast, like guild.join', async () => { + const deps = makeDeps() + const r = await shardIngest.ingest( + { kind: 'guild.leave', id: 7, name: 'The Cartographers', who: '0x2', t: 2 }, deps) + + assert.equal(r.logged, true) + assert.equal(deps.calls.appended.length, 1) + assert.equal(deps.calls.appended[0].kind, 'guild.leave') + assert.equal(deps.calls.broadcast.length, 1) + assert.deepEqual(deps.calls.memberRemove.map((e) => e.who), ['0x2']) +}) diff --git a/server/test/shardVisibility.test.js b/server/test/shardVisibility.test.js index 6a49368..3e53c80 100644 --- a/server/test/shardVisibility.test.js +++ b/server/test/shardVisibility.test.js @@ -116,6 +116,52 @@ test('acct and webId are stripped below admin regardless of feature config', () assert.equal(asAdmin.leader.webId, '42') }) +test('acct and webId are stripped from every member of a guild roster (Protocol 4)', () => { + // A roster is the first frame where the locked fields appear inside an ARRAY of + // actors rather than one nested actor. The walker recurses into arrays, so this + // should already hold — this test is here because it is the difference between a + // public Guilds page listing character names and one publishing 150 account names. + const config = visibility.compileDefaults() + const frame = { + kind: 'guild.roster', + id: 7, + total: 3, + seq: 0, + more: false, + members: [ + { serial: '0x1', name: 'Ada', acct: 'ada_acct', webId: '11', player: true }, + { serial: '0x2', name: 'Bo', acct: 'bo_acct', player: true }, + { serial: '0x3', name: 'Cy', player: true }, // a mobile with no account at all + ], + } + + for (const level of ['anonymous', 'logged_in', 'player', 'staff']) { + const out = visibility.projectFeature('guilds', frame, level, config) + assert.equal(out.members.length, 3, `${level} still sees every member`) + assert.deepEqual(out.members.map((m) => m.name), ['Ada', 'Bo', 'Cy']) + for (const m of out.members) { + assert.equal('acct' in m, false, `${level} must not see a member's acct`) + assert.equal('webId' in m, false, `${level} must not see a member's webId`) + } + } + + const asAdmin = visibility.projectFeature('guilds', frame, 'admin', config) + assert.equal(asAdmin.members[0].acct, 'ada_acct') + assert.equal(asAdmin.members[0].webId, '11') +}) + +test('guild.roster and guild.leave are mapped, so neither falls closed to admin-only', () => { + // Rule 2 fails an unmapped kind closed. That is the right default, but for these + // two it would silently keep the public Guilds page from ever seeing a roster. + const config = visibility.compileDefaults() + for (const kind of ['guild.roster', 'guild.leave']) { + assert.equal( + visibility.kindVisibleTo(kind, 'anonymous', config), true, + `${kind} should reach an anonymous viewer under the default guilds config`, + ) + } +}) + test('a stored rule trying to loosen a locked field is ignored', async () => { withRows([ { feature: 'guilds', enabled: true, audience: 'anonymous', stream: true, fieldRules: { acct: 'anonymous', webId: 'anonymous' } }, @@ -308,10 +354,16 @@ const PRE_V3_PUBLIC_KINDS = [ // pointedly not among them (its feature ships with stream off). const V3_ADDED_PUBLIC_KINDS = ['world.ruleset', 'points.board'] -test('derived PUBLIC_KINDS is exactly the pre-v3 allowlist plus the v3 additions', () => { +// v4 adds guild membership. Both ride the existing `guilds` feature, which is +// already anonymous, so they join the public set — carrying character names and +// serials, never acct/webId, which the locked-field rules strip by suffix even +// inside the roster's member array (see the roster test above). +const V4_ADDED_PUBLIC_KINDS = ['guild.roster', 'guild.leave'] + +test('derived PUBLIC_KINDS is exactly the pre-v3 allowlist plus the v3 and v4 additions', () => { assert.deepEqual( [...visibility.PUBLIC_KINDS].sort(), - [...PRE_V3_PUBLIC_KINDS, ...V3_ADDED_PUBLIC_KINDS].sort(), + [...PRE_V3_PUBLIC_KINDS, ...V3_ADDED_PUBLIC_KINDS, ...V4_ADDED_PUBLIC_KINDS].sort(), ) }) diff --git a/server/utils/shardIngest.js b/server/utils/shardIngest.js index a64d902..b21f6f1 100644 --- a/server/utils/shardIngest.js +++ b/server/utils/shardIngest.js @@ -45,6 +45,12 @@ const LOGGED_KINDS = new Set([ 'server.crashed', // Protocol 2.0: a real-time guild join (the board itself is state, not logged). 'guild.join', + // Protocol 4: the departure counterpart to guild.join, and logged for the same + // reason — it is what a "so-and-so left" feed reads. `guild.roster` deliberately + // stays out: it is board state like guild.update, and it is the one fat frame on + // the wire (~69 bytes per member), so logging it would bloat shard_events with + // a full membership snapshot on every membership change. + 'guild.leave', // Protocol 2.0 provisioning audit (admin channel only — not in PUBLIC_KINDS). 'account.audit', 'account.unlinked', @@ -187,6 +193,16 @@ async function applyStateChange(event, deps) { case 'guild.remove': await shardState.removeGuild(event.id) return + // Protocol 4: membership. A roster arrives in one frame for any realistic + // guild and in several for one over the shard's cap — upsertGuildRoster + // handles both. guild.leave is advisory; the next roster would converge + // anyway, but applying it shows the departure at once. + case 'guild.roster': + await shardState.upsertGuildRoster(event) + return + case 'guild.leave': + await shardState.removeGuildMember(event) + return case 'city.update': // Upserts the board AND captures term history (idempotent). await shardState.upsertGovernor(event) diff --git a/server/utils/shardVisibility.js b/server/utils/shardVisibility.js index 15ac19c..a098569 100644 --- a/server/utils/shardVisibility.js +++ b/server/utils/shardVisibility.js @@ -163,6 +163,13 @@ const KIND_FEATURE = new Map( 'guild.update': 'guilds', 'guild.remove': 'guilds', 'guild.join': 'guilds', + // Protocol 4. Both carry actor data — a roster is an array of actor objects + // and guild.leave names a serial — so they ride the same `guilds` feature and + // the same locked-field rules: `acct`/`webId` inside a roster member are + // stripped below admin by suffix, exactly as `guild.leader.acct` already is. + // Without these two lines rule 2 would fail them closed to admin-only. + 'guild.roster': 'guilds', + 'guild.leave': 'guilds', 'city.update': 'governors', 'presence.online': 'presence', 'region.enter': 'presence',